Bad code exposed! Do you really know when your code smells?

I’ve seen really bad code in my day. I have seen it in legacy code bases, code reviews on greenfield projects, and worse: I’ve even seen it in my own code. In fact, I can open up just about any open source project and immediately detect the existence of potentially bad code.

I am not bragging… it is not as though I am incredibly gifted or that I have superhuman powers. I’ve experienced enough issues from bad code to intuitively pick up on much of it, and the rest of my knowledge comes from reading books on the subject of writing good code. Although I used to detect problem areas on experience alone, I’ve found that having formal names for them aids in identifying and helps in making the case to another developer that something needs to change.

Problem areas are known as ‘code smells’, and it was coined by Kent Beck in explaining to Martin Fowler how he knows when to refactor code. Intuition is still necessary, as the rules for detecting smelly code are presented as guidance. My suggestion is to understand what code smells are and why they are bad, then you can better judge whether source code should be refactored. Sometimes, rules must be broken, but that decision should be made with a clear understanding of the implications. Nearly every time I notice smelly code, a better approach was available.

If you are worried you may have missed something (or even if you are not), download JustCode to see if its state-of-the-art analysis engine picks up anything you should be aware of.

Generalized Smells

Defined code smells are specific about issues, and learning them can help you quickly identify trouble areas. However, not every piece of bad code has been identified and added to a code smell catalog. I am constantly surprised by the number of ways in which developers can write a bad piece of code. Since the ingenuity of developers outpaces one’s ability to catalog, it is useful to know how to spot bad code without resorting to an index.

Fragility

Bottle / John Callow / CC BY

Fragile code breaks due to external changes or untested uses of the system.

Symptoms

• New bugs traced to the subsystem are constantly reported
• Changes to other subsystems break this code’s unit tests
• Developers have begun to call the subsystem a “black hole” or other name indicating a time sink

Impact

The longer the fragile code is allowed to remain, the more time it will cost as a project progresses.

Causes

• The code is not properly separated by concern or responsibility
• Tight-coupling exists between this code and portions of code creating the breaking changes
• Problems were covered up over time instead of fixed
• Natural evolution of the system made it vestigial but still connected to other pieces of code

Solutions

Make sure the subsystem has one concern and the classes involved have a single responsibility. If not, take the time to redesign the subsystem so this is the case. Identify tightly-coupled portions of code and use interfaces to break the coupling. Add dependency injection to make it more flexible and easier to test

If this was an important subsystem that slowly became irrelevant, identify what is currently being used and remove the unused code. There will likely be entanglement with the unused portions of the system, and it may lead to an entire rewrite.

Bandaged systems should be given a total rewrite; applying more bandages won’t fix anything.

Poor Communication

scream and shout / Mindaugas Danys / CC BY

Software is developed to serve a purpose, and to continue to serve that purpose they must be maintained. Code that poorly communicates what is doing and its intent makes software difficult to maintain.

Symptoms

• Newly-hired, experienced developers take a long time to get up to speed on the system
• Developers avoid fixing bugs in code other than their own
• A developer is considered valuable because that developer is the only one who knows a particular system

Causes

• Lack of coding standards or failure to observe them
• Lack of ubiquitous language or failure to utilize it
• Lack of documentation
• Lack of unit tests
• Complex code
• Developed using languages or tools the developers did not understand

Acceptable Scenarios

Optimizing will often make code more complex. This is okay as long as it is documented so other developers understand why the code is less understandable.

Solutions

Widespread problems must be addressed by making changes at the team or organization level. This includes creating coding standards, implementing testing requirements, and so on. Code bases will require cleaning afterwards, and the decision should be made whether to do it all at once or over time.

Smaller issues should be fixed as they’re encountered: clarify names, simplify complex code (or encapsulate it), apply standard programming idioms, etc.

Duplication

the rhythm of repetition / eren {sea+prairie} / CC BY

Every piece of knowledge should have one representation. Duplication makes a system more difficult to change as that discrete piece of knowledge has multiple representations throughout the system.

Symptoms

• A change requires making an exact, or near exact, change elsewhere
• A change occurs but users report the old behavior still exists

Causes

• Copy & Paste
• Failure to refactor near identical code
• Failure to identify behavior in another subsystem

Solutions

Some code duplication is easy to identify and correct with common refactorings. Other forms of duplication may have identical logic with a different implementation. Having a solid unit testing framework with a ubiquitous language to describe software specifications will make these easier to identify.

Rigidity

Broken / Gary Hymes / CC BY

Rigid code is resistance to change. Change it, and you have to change something else. Change something else, and you have to change it.

Symptoms

• Simple changes affect many pieces of code

Causes

• Tight-coupling
• Low cohesion

Solutions

This code must be made more flexible. Decompose into smaller classes and interfaces. You can use the rigid code’s collaborators to identify the appropriate interfaces to use.

Cleaning Your Code

Using the generalized smells can help you identify potential problem areas. It is still good to learn more specific smells from books such as Refactoring, as they provide guidance on specific refactorings to perform.

You can also use tools such as Telerik JustCode to quickly detect issues. JustCode identifies problem areas automatically and marks them. If a known fix is available, the quick fix menu will contain the fix so you can quickly refactor your code.

For code cleaning issues that can’t be detected automatically, such as cohesion problems, JustCode provides 30+ intelligent refactorings to automate the work typically done by hand.

Chris is a technical evangelist for JustCode and a C# MVP. He's always sure to bring a can of deodorizer to a legacy code base.