Code Smells

Code smells describe code that complicates, duplicates, bloats or tightly couples code.

Code smells were first described by Kent Beck.

Books describing / relating to code smells:

• Refactoring: Improving the Design of Existing Code by Martin Fowler, Kent Beck, John Brant, William Opdyke, Don Roberts
  
• Refactoring to Patterns by Joshua Kerievsky
  

Useful sites covering code smells:

• Canterbury University's code smells and refactoring wiki

Common types of code smells:

Dead code

Code that is no longer used.

The Problem

• Harder to comprehend the code base.
• Wasted time - reading through, or even changing dead code.
• More dead code - monkey see, monkey do.

The Solution

Remove dead code. The following factors will help in doing so:

• Domain knowledge - to ensure that the code is in fact dead.
• Tools - is the code called from anywhere?
• Testing coverage - can tests be run after deletion to ensure functionality still works?

Version control will also help ensure that code can be restored if accidentally deleted.

Duplicate code

Code that is the same, or performs the same function are showing up in multiple places within a program.

The Problem

• Bloat - more code will increase the size of classes, making them more time consuming to read and understand.
• Maintenance - changes to one of the blocks of code will most likely cause other blocks of code to also be changed. This can also lead to greater defects if the duplicate code isn't updated.

The Solution

Refactor the code, combine duplicate code into a method, or into separate classes.

Comment

Comments are being used to describe code, as the code readability of the code is poor, making the code hard to follow and understand.

The Problem

• Bloat - commented code is often bloated code, making it harder to understand what's going on.
• Understanding - commented variables can be a sign that the variable name doesn't describe well enough what it is.

The Solution

Name things sensibly, and break the problem down.


Spell variables out rather than using crytpic acronyms.


If a piece of code is complex, try to split the code into other methods or classes.

Long Method

A method is too long

The Problem

• Readability - longer code takes longer to read and comprehend.
• Reusability - a long method may be able to be split into methods or classes that can then be reused.
• Testability - a long method is likely to need more testing.

The Solution

Break the method up into a greater number of methods, or into other classes. Have one method represent a single function, and one class a single concept.

It may look like breaking a method up into many methods may cause performance degradation, but given the advancement in compilers, there may be no performance hit at all.

Long Parameter List

A method has too many parameters

The Problem

• The method is doing too much - why does it need all of that information?
• Understandability - lots of parameters will make code harder to understand.

The Solution

Put the data into their own classes, and possibly break the method up into several methods. Ensure that each method is performing a single function, and each class modelling a single concept.

Large Class

A class has taken on too much responsibility, and is modelling within it more than one concept.

The Problem

• Complexity - if your class represents more than a single concept, it will be harder to understand.
• Bloat - a greater number of fields and methods will mean the class will take longer to understand.

The Solution

Ensure that the class represents a single concept, and break it into several classes if it represents more than a single concept.


You may find that functionality or state could belong to a different class, in which case functionality or state could be moved to the other class.

Primitive Obsession

Code is using primitive data types and method calls to generate desired outcomes, and could be written in a more descriptive and sustainable way. E.g. when generating XML, a primitive obsession smell would be code that writes each tag out line by line. The solution would be to encapsulate the concept of a Tag, and the Tag's attributes into classes, and to render the XML from the classes instead.

The Problem

• Code written is not reusable.
• It may be less clear to someone reading the code what is going on.

The Solution

Encapsulate data concepts in classes, and create and call methods on objects of a class to attain desired results.

Speculative Generality

Creating today what we speculate will be needed in the future (where there is no current need for a particular feature / design).

The Problem

• Wasted time creating code that isn't actually going to be used (it is dead).
• Greater complexity - now there is more code to sort through

The Solution

Design code to a specification. If a feature is needed in the future, you can always refactor code if need be. Until that time, remember YAGNI (You ain't gonna need it!).