When your code smells it is often not awful code that makes your hold your nose. Rather it’s a simple indication that something may need to be changed. However when you don’t change and ignore you probably will end with a project of unmaintainable code. So you better learn to change smelling code. This is what Kent Beck and Martin Fowler did in their Refactoring book. They created a list of smells and what to do about them.
General Code Smells
Here is a list of common code smells for Objective-C and how to fix them. Most of them are language independent but there are also special Objective-C smells.
Duplicated Code
Number one in the stink parade is duplicated code. If you see the same code structure in more than one place, you can be sure that your program will be better if you find a way to unify them.
Extracting this code into methods and invoke the code from both places. Another common duplication problem is when you have the same expression in two sibling subclasses. You can eliminate this duplication by using extract method in both classes and pull up the method to their superclass. If you have duplicated code in two unrelated classes consider using to extract the code in its own class.
Long Methods
The object oriented programs that live best and longest are those with short methods. The longer a procedure is, the more difficult it is to understand and test. And if you have a good name for a method you don’t need to look at the body and are much faster in reading code. 99% of the time, all you have to do to shorten a method is extract method. Find parts of the method that seem to go nicely together and make a new one. Long lists of parameter objects can be slmmed down by parsing whole objects or introduce parameter Objects.
Large Class
When a class is trying to do too much it often shows up as too many instance variables. When a class has too many instance variables, duplicated code cannot be far behind. Delegation is often the solution to the problem extract to a class or subclass. In Objective-C and Cocoa this can be done with the well known delegation pattern. If your class is a GUI class you may need to make data and behaviour to a separate domain object.
Long Parameter List
In the “Pascal Programming Days” we were taught to pass in everything as parameters. This was understandable because the alternative was global data, and global data and state is really evil, Objects change this situation because if you don’t have something you need, you can always ask another object to get it for you. So you only pass the object. Use the whole Object and preserve it or introduce Parameter Objects.
Divergent Change and Shotgun Surgery
Divergent change occurs when one class is commonly changed in different ways for different reasons. To clean this up you identify everything that changes for a particular class and use Extract Class to put them together. Shotgun surgery is similar to divergent change but is the opposite. You whiff this when every time you make a kind of change, you have to make a lot of little changes to a lot of different classes. Then the changes are all over the place they are hard to find, and it’s easy to miss an important change. In this case you will move methods and fields to put all the changes into a single class.
Switch Statements
One of the most obvious symptoms of object-oriented code is its comparative lack of switch statements. The problem with switch statements is essentially that of duplication. Often you find the same switch statement scattered about a program in different places. If you add a new clause to the switch, you have to find all the others. Most time you see a switch statement you should consider polymorphism. Often the switch statement switches on a type code. So extract the switch statement into methods and then move the methods to get it onto the class where the polymorphism is needed.
Message Chains
You see message chains when a client asks one object for another object, which the client then asks for yet another object, which the client then asks for yet another another object, and so on. Navigating this way means the client is coupled to the structure of the navigation and all queried objects. Any change to the intermediate relationships cause the client to have to change. Extensive use of message chains is the best sign that the Law of Demeter has been broken. In particular, an object should avoid invoking methods of a member object returned by another method. Instead write a wrapper method that calls this method. Often a better alternative is to see what the resulting object is used for. See weather you can use extract method to take a piece of the code that uses it and then move method to push it down the chain.
Delegation abuse
One of the prime features of objects is encapsulation. Encapsulation often comes with delegation. However this can go too far. When you look at a class’s interface and find half the methods are delegating to another class. After a while it is better to talk to the object that really knows what’s going on. If only a few method aren’t doing much, use inline method to inline them into the caller.
Comments
Comments are often used when code becomes wired. So Comments can lead us to bad code. When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous. If your comment explains why a block of code does try to extract the block into its own method. If the method is already extracted but you still need a comment to explain what it does, use rename method. If you need to state some preconditions about the required state use assertions.
