Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. General Programming
  3. C#
  4. Is this good code practice?

Is this good code practice?

Scheduled Pinned Locked Moved C#
helpquestion
39 Posts 10 Posters 6 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • L Lost User

    It gives you speed when compared to overrides.. a switch is much faster though (and in this case probably also clearer) That doesn't mean I disagree, of course.

    realJSOPR Offline
    realJSOPR Offline
    realJSOP
    wrote on last edited by
    #5

    But we're not talking about overrides. We're talking about populating a dictionary from a list...

    .45 ACP - because shooting twice is just silly
    -----
    "Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
    -----
    "The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001

    L 1 Reply Last reply
    0
    • V venomation

      I used the following code in a programming lesson to help me with a project but the lecturer says it is hard to maintain although I made it like that for better manageability:

          static List \_person = new List();
          static void Main()
          {
              Dictionary methodList = new Dictionary();
      
              methodList.Add("1", (() =>
                  \_person.ForEach(s => Console.WriteLine("{0} {1}", s.Name, s.Title))));
      
              \_person.Add(new Person( "James","Prog" ));
              \_person.Add(new Person( "Claire","Illu"));
      
              Console.WriteLine("Press 1 to show all data or 2 to quit");
      
              string input = Console.ReadLine();
              if (methodList.ContainsKey(input)) methodList\[input\]();
      
              Console.Read();
      
          }
      
      P Online
      P Online
      PIEBALDconsult
      wrote on last edited by
      #6

      Did your XML generics get swallowed? What exactly is he concerned about?

      V 1 Reply Last reply
      0
      • V venomation

        I can see your point however I disagree about lambda being a hindrance, the lambda expression in there is not complicated and saves me the hassle of having to make a method just to display the members which may only happen once. Although I can see that if I needed to display the details in other classes it would not be very productive. Thanks for the reply ! ;)

        realJSOPR Offline
        realJSOPR Offline
        realJSOP
        wrote on last edited by
        #7

        We're talking about maintenance, not productivity. If you want better maintainability, you shouldn't use the lamda. Adding code complexity with no tangible performance gain is bad practice, IMHO.

        .45 ACP - because shooting twice is just silly
        -----
        "Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
        -----
        "The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001

        1 Reply Last reply
        0
        • realJSOPR realJSOP

          But we're not talking about overrides. We're talking about populating a dictionary from a list...

          .45 ACP - because shooting twice is just silly
          -----
          "Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
          -----
          "The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001

          L Offline
          L Offline
          Lost User
          wrote on last edited by
          #8

          I know, but soon the "anti switch" guerrillas will find the thread and suggest "a beautiful OO solution" :)

          realJSOPR 1 Reply Last reply
          0
          • L Lost User

            I know, but soon the "anti switch" guerrillas will find the thread and suggest "a beautiful OO solution" :)

            realJSOPR Offline
            realJSOPR Offline
            realJSOP
            wrote on last edited by
            #9

            Don't you just hate zealots? :)

            .45 ACP - because shooting twice is just silly
            -----
            "Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
            -----
            "The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001

            P P 2 Replies Last reply
            0
            • P PIEBALDconsult

              Did your XML generics get swallowed? What exactly is he concerned about?

              V Offline
              V Offline
              venomation
              wrote on last edited by
              #10

              Thanks for the replies so far! I think my lecturer is assuming that the program that was shown could be "grown" so I did not use switch as I assumed if the system were to grow adding more switch statements would decrease its quality? Is there a better way around this instead of encapsulating the routines into the dictionary?

              L 1 Reply Last reply
              0
              • V venomation

                Thanks for the replies so far! I think my lecturer is assuming that the program that was shown could be "grown" so I did not use switch as I assumed if the system were to grow adding more switch statements would decrease its quality? Is there a better way around this instead of encapsulating the routines into the dictionary?

                L Offline
                L Offline
                Lost User
                wrote on last edited by
                #11

                More switches? Why not just add more cases to a big switch?

                V 1 Reply Last reply
                0
                • L Lost User

                  More switches? Why not just add more cases to a big switch?

                  V Offline
                  V Offline
                  venomation
                  wrote on last edited by
                  #12

                  That is true I initially used switches but as the project has an "unknown" amount of lookups I thought the latter approach would suit it?

                  P L 2 Replies Last reply
                  0
                  • realJSOPR realJSOP

                    Don't you just hate zealots? :)

                    .45 ACP - because shooting twice is just silly
                    -----
                    "Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
                    -----
                    "The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001

                    P Online
                    P Online
                    PIEBALDconsult
                    wrote on last edited by
                    #13

                    "I know that there are people who do not love their fellow man, and I hate people like that!" -- Tom Lehrer

                    R 1 Reply Last reply
                    0
                    • V venomation

                      That is true I initially used switches but as the project has an "unknown" amount of lookups I thought the latter approach would suit it?

                      P Online
                      P Online
                      PIEBALDconsult
                      wrote on last edited by
                      #14

                      Perhaps you should have told us what the requirements were. What would he say to a system of plug-ins? :cool:

                      1 Reply Last reply
                      0
                      • V venomation

                        That is true I initially used switches but as the project has an "unknown" amount of lookups I thought the latter approach would suit it?

                        L Offline
                        L Offline
                        Lost User
                        wrote on last edited by
                        #15

                        Where does that "unknown" come from? Unknown at compile time? (that would eliminate switch as option..)

                        V 1 Reply Last reply
                        0
                        • L Lost User

                          Where does that "unknown" come from? Unknown at compile time? (that would eliminate switch as option..)

                          V Offline
                          V Offline
                          venomation
                          wrote on last edited by
                          #16

                          It relates to an "unknown" GUI requirement as we have not been told what options we will provide the user with so options 1 and 2 are there for show.

                          L 1 Reply Last reply
                          0
                          • V venomation

                            I can see your point however I disagree about lambda being a hindrance, the lambda expression in there is not complicated and saves me the hassle of having to make a method just to display the members which may only happen once. Although I can see that if I needed to display the details in other classes it would not be very productive. Thanks for the reply ! ;)

                            P Online
                            P Online
                            PIEBALDconsult
                            wrote on last edited by
                            #17

                            A simpler anonymous method may suffice.

                            1 Reply Last reply
                            0
                            • V venomation

                              It relates to an "unknown" GUI requirement as we have not been told what options we will provide the user with so options 1 and 2 are there for show.

                              L Offline
                              L Offline
                              Lost User
                              wrote on last edited by
                              #18

                              Well if it's just that you could just go back and add some cases.. right?

                              V 1 Reply Last reply
                              0
                              • L Lost User

                                Well if it's just that you could just go back and add some cases.. right?

                                V Offline
                                V Offline
                                venomation
                                wrote on last edited by
                                #19

                                Ok thanks for all the comments, it appears that a switch is all that is needed :-D

                                1 Reply Last reply
                                0
                                • V venomation

                                  I used the following code in a programming lesson to help me with a project but the lecturer says it is hard to maintain although I made it like that for better manageability:

                                      static List \_person = new List();
                                      static void Main()
                                      {
                                          Dictionary methodList = new Dictionary();
                                  
                                          methodList.Add("1", (() =>
                                              \_person.ForEach(s => Console.WriteLine("{0} {1}", s.Name, s.Title))));
                                  
                                          \_person.Add(new Person( "James","Prog" ));
                                          \_person.Add(new Person( "Claire","Illu"));
                                  
                                          Console.WriteLine("Press 1 to show all data or 2 to quit");
                                  
                                          string input = Console.ReadLine();
                                          if (methodList.ContainsKey(input)) methodList\[input\]();
                                  
                                          Console.Read();
                                  
                                      }
                                  
                                  A Offline
                                  A Offline
                                  AspDotNetDev
                                  wrote on last edited by
                                  #20

                                  If you need that logic in multiple places, there might be some value in putting it in a dictionary structure (e.g., if you need a combination of the values and the logic for selecting which keys depends on where the dictionary is used). That way, you can pass the dictionary around. Then again, you could just wrap a switch statement in a delegate and pass that around (may not work well if there is custom logic to access several keys). In this specific case, I see no reason a switch statement should not be used. It is certainly simpler and I recommend choosing the simpler approach unless there is a real chance you will need the more complex approach later.

                                  [Forum Guidelines]

                                  1 Reply Last reply
                                  0
                                  • V venomation

                                    I used the following code in a programming lesson to help me with a project but the lecturer says it is hard to maintain although I made it like that for better manageability:

                                        static List \_person = new List();
                                        static void Main()
                                        {
                                            Dictionary methodList = new Dictionary();
                                    
                                            methodList.Add("1", (() =>
                                                \_person.ForEach(s => Console.WriteLine("{0} {1}", s.Name, s.Title))));
                                    
                                            \_person.Add(new Person( "James","Prog" ));
                                            \_person.Add(new Person( "Claire","Illu"));
                                    
                                            Console.WriteLine("Press 1 to show all data or 2 to quit");
                                    
                                            string input = Console.ReadLine();
                                            if (methodList.ContainsKey(input)) methodList\[input\]();
                                    
                                            Console.Read();
                                    
                                        }
                                    
                                    A Offline
                                    A Offline
                                    Abhinav S
                                    wrote on last edited by
                                    #21

                                    You might want to move some of this code out of your main method and into other methods. For e.g.

                                    thebuzzwright wrote:

                                    methodList.Add("1", (() => _person.ForEach(s => Console.WriteLine("{0} {1}", s.Name, s.Title))));

                                    might go into a readInput() method.

                                    thebuzzwright wrote:

                                    person.Add(new Person( "James","Prog" )); _person.Add(new Person( "Claire","Illu"));

                                    could go into loadData() or something like that.

                                    thebuzzwright wrote:

                                    string input = Console.ReadLine(); if (methodList.ContainsKey(input)) methodList[input]();

                                    could go into a method called userResponse(). If any of these methods are modified later, these logical groups might make it easier for others to understand what is done where in the program.

                                    modified on Saturday, May 1, 2010 12:39 AM

                                    1 Reply Last reply
                                    0
                                    • P PIEBALDconsult

                                      "I know that there are people who do not love their fellow man, and I hate people like that!" -- Tom Lehrer

                                      R Offline
                                      R Offline
                                      Roger Wright
                                      wrote on last edited by
                                      #22

                                      PIEBALDconsult wrote:

                                      Tom Lehrer

                                      One of my heroes!

                                      "A Journey of a Thousand Rest Stops Begins with a Single Movement"

                                      1 Reply Last reply
                                      0
                                      • V venomation

                                        I used the following code in a programming lesson to help me with a project but the lecturer says it is hard to maintain although I made it like that for better manageability:

                                            static List \_person = new List();
                                            static void Main()
                                            {
                                                Dictionary methodList = new Dictionary();
                                        
                                                methodList.Add("1", (() =>
                                                    \_person.ForEach(s => Console.WriteLine("{0} {1}", s.Name, s.Title))));
                                        
                                                \_person.Add(new Person( "James","Prog" ));
                                                \_person.Add(new Person( "Claire","Illu"));
                                        
                                                Console.WriteLine("Press 1 to show all data or 2 to quit");
                                        
                                                string input = Console.ReadLine();
                                                if (methodList.ContainsKey(input)) methodList\[input\]();
                                        
                                                Console.Read();
                                        
                                            }
                                        
                                        S Offline
                                        S Offline
                                        Som Shekhar
                                        wrote on last edited by
                                        #23

                                        I see the point your Lecturer is trying to convey. You are at a learning stage. Hence there will always be possibilities that the code you write will need to be improved/modified/tweaked. If this is the final code piece and there cannot be any more changes to be made, this code is fine. However if there is a possibility that someone else may be using your code (as in the case of Open Source/ or in a Production Environment), this code may cause misunderstandings. Consider this, what happens if you join a company and are given this piece of code and are asked to make some changes. How comfortable would you be? (You understand this code. But any other such code example?)

                                        1 Reply Last reply
                                        0
                                        • realJSOPR realJSOP

                                          Don't you just hate zealots? :)

                                          .45 ACP - because shooting twice is just silly
                                          -----
                                          "Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
                                          -----
                                          "The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001

                                          P Offline
                                          P Offline
                                          Pete OHanlon
                                          wrote on last edited by
                                          #24

                                          You rang?

                                          "WPF has many lovers. It's a veritable porn star!" - Josh Smith

                                          As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.

                                          My blog | My articles | MoXAML PowerToys | Onyx

                                          1 Reply Last reply
                                          0
                                          Reply
                                          • Reply as topic
                                          Log in to reply
                                          • Oldest to Newest
                                          • Newest to Oldest
                                          • Most Votes


                                          • Login

                                          • Don't have an account? Register

                                          • Login or register to search.
                                          • First post
                                            Last post
                                          0
                                          • Categories
                                          • Recent
                                          • Tags
                                          • Popular
                                          • World
                                          • Users
                                          • Groups