Worst-Smelling Code

I love cleanliness.  I can say that, even as the far reaches of my desk have accumulated an unhealthy layer of dust.  What I'm really talking about here is code.  I have an affinity for clean, concise, logically separated code.  Which means, as you can imagine, when I see messy, duplicated, bloated code, I feel like throwing something (or someone).

In the software development world, we call this "Code Smell".  I wanted to share what I consider to be some of the smelliest (think bad cheese) code.  Keep your code linen fresh, and avoid these:

Duplicated Code.  Sure, it may be a lot faster and easier to just copy those 20 awesome lines of code, and use it everywhere else you need the same logic.  But, you're just hurting yourself (or whoever has to maintain the code).  The disadvantages of copy / paste are obvious: if you need to change the logic due to a business rule change, you'll end up having to change it in all the places you copied it.  However, there are other forms of duplication, where code is copied and then tweaked.  This is even worse, as now you need to multiply the difficulty of reading the code by the number of times it's copied and tweaked.  In this case, for goodness' sake, create a method with parameters!

God Objects.  A "god object" is an object that knows or does too much.  We've all seen examples of this (and are probably guilty of doing it) in the real world.  One of the worst offenders I've seen is a classic ASP app where there was a single .asp page, whose single code path was a huge case statement which, depending on the combination of querystring variables (all integers) would decide where to navigate or what logic to run. It also handled any session variables and caching.

Aside from being a debugging nightmare, god objects go against the time-tested object-oriented tenets of "single responsibility", decoupling and separation of concerns.

My suggestion for breaking up an existing god object is to start from scratch. Document the requirements and properly separate functionality.  Think high cohesion, and low coupling.

Methods with too many parameters.  This is another easy way to create a debugging debacle and maintenance nightmare.  In my opinion, when you get to around 5 parameters, you may want to rethink your approach.  A couple of options would be to 1) create a struct or class to pass instead of the parameter list or 2) rethink the method altogether and consider splitting it up into multiple methods.

Contrived Complexity (See KISS).  Too often, developers either try to follow a pattern which isn't suited for what they're building, or they attempt to create elaborate "frameworks" to make their code flexible to the Nth degree, when the Bth or Cth degree is all that's really needed.  YAGNI is another rule of thumb that directly applies here (You aren't gonna need it).  If we try to anticipate what we might need 2 or 3 years from now and bake it into our code, we create unnecessary complexity, which means code with more room for error and more difficult to test and maintain.  Please, build what you need when you need it, and allow the software to evolve naturally in an agile manner.

Go get rid of that stench!