Long Methods and Classes Are Evil

I’ve spent a great deal of time in the last week reverse engineering legacy VB6 code, and I’d like to rant a little bit. What’s making my life harder is how large and complex the methods are in each class module. The code is very procedural, with little usage of subroutines. The VB functions are several pages long, if/then nesting goes 6 or 7 deep, and case statements span hundreds of lines. Unsurprisingly, the code is hard to maintain and brittle in regression testing. Finding the location of a piece of functionality requires arcane knowledge of the application or some amount of luck with CTRL-F.

If the application you are building or modifying is important or useful, it is a sure bet the code will have to be modified later as requirements change and problems occur. There are some easy steps to take to make your code more understandable. Good naming is obviously important, a variable named “a1″ isn’t very descriptive, especially when it is an untyped variant. Can you say “Mystery Meat?”

My personal pet peeve is really long methods and classes that do way too much. I’m a terrible slob in most other ways, but I want my code readily organized with each type of code on its proper shelf, err, class. The human brain can only process so much information at a time. Small methods can be seen and understood all at one time. Bugs can hide out in very long methods because there is simply more brush to hide in. Large case statements are veritable camouflage for lurking bugs.

Rules of Thumb for Class and Method Size

1. Any method should fit into your IDE window so the entire method can be read at one time. I’d take it farther if you can. One of the few code metrics I pay attention to is the average lines of code per method. I strive for an average of not more than 5 lines of code per method.
   
2. Too many private members. You know what I mean, if you see a class with an excessive number of private fields (say >10), the class is probably too big. I’m tracing a class with about 75 private fields this morning. Not pleasant. In Fowler’s Refactoring book this is described as the code smell “Primitive Obsession.”
   
3. The size of the code file. Offhand, I’d say any class over 10k in size is getting too large for its britches. A class that is 200k in the file system is certainly too long. TestFixture classes might be an exception.
   
4. Exception handling code and instrumentation tend to push methods to be much larger. Invest some thought in how to segregate this type of code away from the main functionality. Aspect Oriented Programming may justify its existence by merely doing this.
   
5. Reduce the number of public methods in a class. Just picking a number, I would say less than 10 in most cases. There is a good discussion on this subject at http://c2.com/cgi/wiki?FewShortMethodsPerClass.

I’m not just picking on VB developers, because I’ve seen (and written) bad examples in other languages, including a C# system that is featured in an early case study on MSDN as a shining example of the wonders of .NET. At a former employer, my teammates and I inherited a large new supply chain system hacked together by a very prominent consultancy. I distinctly remember a coworker printing out a Perl script that contained an if/then branch spanning 18 pages.

As an aside I’m 2-3 years removed from any kind of serious VB6 work. I remember a time when I was a staunch pro-VB guy because of its productivity for building code rapidly and hated being looked down on by Java guys. After becoming quite used to C# coding with ReSharper and other toys, VB6 feels extremely clunky. Combining well-factored C# code with the advanced code navigation features from VS.NET itself and ReSharper, leads to code that is readily traced and reverse-engineered. Oh yeah, I forgot to mention that variants suck and COM sucks more. I do still hate being looked down on by Java guys, though.