Steve Maine writes an excellent post on "code smells".
Things that I try to keep a look out for:
- Code that seems too familiar. If it seems familiar, it probably means I've already written something similar elsewhere and should think about refactoring to eliminate duplication.
- Functions that scroll off the screen. If my function can't fit on one screen, it's probably doing more than one thing. Time to think about splitting it up.
- Nested “if” statements. Conditional logic is inevitable, but a well-factored solution doesn't rely on a lot of it to get the job done. If I'm nesting a lot of if/else if/else statements, I'm not thinking correctly. This stems from the general thinking that good code has a low cyclomatic complexity.
- Functions that take many parameters.A good object-oriented solution keeps state around in object-level variables, not parameters. Chances are, if I'm passing a lot of parameters to a function I should think about introducing an encapsulating abstraction to reduce complexity.
- Classes that have many public members. A good class abstraction serves a specific and defined purpose. If I see a class that exposes a multitude of functions via its public interface, it's time to go back and look for potential duplication. Chances are, some of these API's are redundant or unnecessary.
- Variables, functions, or types that have awkward names. This one stems from a conversation I had with Brian Jackson a while back. He made the observation that the elegance of his code seemed to be directly indicated by the ease he had in naming his functions. Good functions should do one thing and do it well. When they're written this way, it's very easy to come up with a meaningful function name -- it obvious what the function should be called. Same thing with variables and classes. Thus, if I find myself scratching my head trying to come up with a name for a function I'm implementing, it's usually a good indication that I need to rethink what I want that function to accomplish.
