• Visitors can check out the Forum FAQ by clicking this link. You have to register before you can post: click the REGISTER link above to proceed. To start viewing messages, select the forum that you want to visit from the selection below. View our Forum Privacy Policy.
  • Want to receive the latest contracting news and advice straight to your inbox? Sign up to the ContractorUK newsletter here. Every sign up will also be entered into a draw to WIN £100 Amazon vouchers!

Code reviews

Collapse
X
  •  
  • Filter
  • Time
  • Show
Clear All
new posts

    #91
    Originally posted by SpontaneousOrder View Post
    I agree with everything you said there, even though I go for small methods. I aim for single responsibility - at both class and method level. I don't think keeping methods small, per se, helps testability, because i factor the large methods out into smaller private methods - and I don't unit test private methods.

    Class level single responsibility, though, tends to make testing easier as a side effect. Single reason to change = helpful, easier to test = bonus!

    +1 on the not designing code for testability though (apart from special cases).
    I very rarely ever write private methods and in fact I would have them banned, I cant see the point of splitting up a large public method into smaller private methods. It does make the readability of the code much worse as you have to use the IDE then to follow the code looking for calls rather than moving down one large method.

    Comment


      #92
      Originally posted by minestrone View Post
      You changed the classes, created new class signatures and therefore created a new internal API. If you cant see that is a redesign then no wonder the permies are despairing of you.
      That's like an editor formatting a manuscript into appropriate paragraphs with proper grammar, and calling it a rewrite. It's not a redesign if no logic changes. It's just a tidy up.

      Comment


        #93
        Originally posted by minestrone View Post
        I very rarely ever write private methods and in fact I would have them banned, I cant see the point of splitting up a large public method into smaller private methods.
        Well by making them private you're highlighting the public ones as being the one's that client code should be using.

        Originally posted by minestrone View Post
        It does make the readability of the code much worse as you have to use the IDE then to follow the code looking for calls rather than moving down one large method.
        Oh... I see.
        I hope to god I never have to work on anything you've been near :P

        Comment


          #94
          Originally posted by SpontaneousOrder View Post
          That's like an editor formatting a manuscript into appropriate paragraphs with proper grammar, and calling it a rewrite. It's not a redesign if no logic changes. It's just a tidy up.
          You clearly think of code as a sequential list of lines like words in a book.

          Comment


            #95
            Originally posted by SpontaneousOrder View Post
            I hope to god I never have to work on anything you've been near :P
            I'll look out for your CV gentile.

            Comment


              #96
              Originally posted by minestrone View Post
              Methods should be sized to do a specific task, dogmatically designing your code based on LOC is wrong, as is designing your class for ease of testing. The cleanest code comes from clarity of the interface and the worst thing you can do for that is create methods based on size and methods that make it easier to get your lines of code tested percentage up.
              Erm not sure that's anything to do with my post. I never said a method had to be a specific size, nor to write code to fit testing. I gave a number of reasons why long methods can be problematic and why there are good reasons to address this.

              Comment


                #97
                Originally posted by minestrone View Post
                I very rarely ever write private methods and in fact I would have them banned, I cant see the point of splitting up a large public method into smaller private methods. It does make the readability of the code much worse as you have to use the IDE then to follow the code looking for calls rather than moving down one large method.
                Really, you can't see any good reason to split a large public method, using private methods? What about making the code easier to read and understand, especially for the poor bugger that has to maintain your code. If you have a well named private method you instantly know what that bit of code does and you can choose to ignore it or investigate it when debugging.

                What about re-using code, perhaps in the same class. Would you have two public methods using the same private method or would you cut and paste and duplicate the code in both public methods?

                I'm trying really hard to understand your perspective but this is basic stuff and you seem to have not grasped it at all.

                Comment


                  #98
                  Originally posted by woohoo View Post
                  Erm not sure that's anything to do with my post. I never said a method had to be a specific size, nor to write code to fit testing. I gave a number of reasons why long methods can be problematic and why there are good reasons to address this.
                  I would guess that some Uncle Bob acolyte, who's missed the point entirely, has poisoned the well; and minestrone has spent too long drinking from it.

                  Comment


                    #99
                    Originally posted by minestrone View Post
                    I very rarely ever write private methods and in fact I would have them banned, I cant see the point of splitting up a large public method into smaller private methods. It does make the readability of the code much worse as you have to use the IDE then to follow the code looking for calls rather than moving down one large method.
                    Strange. Do you never have loops with logic inside? Or a number of sequential steps that need to be carried out? Either of these are candidates for using a private method - precisely to make the code more readable.

                    tulip, that's even what we did in procedural programming, before OO was invented!
                    Down with racism. Long live miscegenation!

                    Comment


                      If you have one 100 line method any reusable elements should be placed into a reusable public method in an object itself, I rarely If ever have an object call methods on the same object, again I don't have methods call private methods.

                      If you can do that it is worse taking a 100 line method and splitting it into 10 10 line private methods gives you 110 lines of code and 11 methods, you have taken a clean interface with one method and turned it into a mess of 11 methods, to understand the code takes longer as code should be understood reading the interface not the code. You have leaked what was a less than ideal 100 line method but one that was encapsulated into poor API. Interface should always be king.

                      Clearly you client has seen what you have done and though the same, else you would not be on here pissing your knickers about getting slated at code review time and looking for some affirmation which you only seem to be getting from the assorted cretins clique of CUK.

                      Comment

                      Working...
                      X