RE: bad smell code # 2 Long Method

I took a look at a codebase reviewed before and I found that the top 3 longest methods are as following:

Interface. InsertNADAData(String) As Boolean ( 774 LOC)

 

RegistrationNewUserControl. ValidateControls(Integer) As Boolean ( LOC 629)

 

RenewalRegistrationController. VehicleDetails(String, String, String, String, FormCollection, String, String, String, String, String, String, String, String, String, String, String, String, String, String) As ActionResult ( LOC 617)

 

If you happen to step into any of these members, you would agree with me when I say these code blocks are smell bad. They are very difficult to understand let alone maintaining them or extend them….

 

Let’s say how the experts say about long methods:

 

The object program that lives best and longest is these with short methods. Programmers new to objects often feel that no computation ever takes places that object programs are endless sequence of delegations. When you have lived with such a program for a few years, however, you learn just how valuable are those little methods are.  All of these payoffs of indirection – explanation, sharing and choosing. All supported by little methods. …. Since the early days of programming, people have realized that the longer the procedure, the more difficult to understand.

 

Okay, we accepted the call that “short method is good, long method is bad” but given the code base we have, how we gradually shorten the methods of thousands class in our code base?

 

.99% of the time, all you have to do to shorten a method is Extract. Find a part of the method that seems to go nicely together and make it a new method.

 

.If you find that you have a method with lots of parameters, you can introduce parameter objects to group these parameters into groups. (I am sure you would like to try this method on the third example above)

 

.If you find that you have a method with lots of temporary variables, you can move the related code block from your method to the class that is close to it as a read only property or method. (I am sure you have seen many of such examples from my code review comments)

 

After you have done all above, if you still end up with a long method with lots of parameters and local variables, it is time to turn the method into a class by itself. After you do that, all you original local variable become fields of the new class, then you can decompose the method into other small methods on the same object.

 

How do you identify the clump of code to extract? A good technic is to look for comments. They often signal this kind of semantic distance. A block of code with a comment that tells you what it is doing can be replaced by a method whose name is based on the comments. Even a single line is worth extracting if it needs explanation.

 

Conditionals and loops also give signs for extractions. If you have a complicated conditional (if-then-else) statement, extract methods from the <condition part>, <then part> <else part>. With loops, extract the loop and the code within the loop in its own method.