Code Smells : Dispensable

Continuing on our discussion on Code Smells and Refactoring techniques, we will discuss Dispensable in this post.

Dispensables are avoidable components whose absence would make the code much more cleaner, readable and efficient.

Comments

A joke is not a good one if needed to be explained. A similar philosophy holds for code as well. If you feel that the code requires comments to understand, then it is better to refactor  it such a way that it eliminates the need of comments.

You could start with refactoring techniques like Rename Method, Rename Variable, along Extract Variable and Extract Method. Quite often using asserts (introduce assertions) can also aid in increasing the readability of the code.

Duplicate Code

When you have a larger team, there is a distinct possibility that two developers working different parts of the project might end up developing similar/same method. The similarity could could be subtle as well, where similarity is visible to only in certain parts of code, while performing the same job.

Extract Method, Extract superclass and Extract class are some of the refactoring methods that can be applied. If two methods do same job but using different algorithms, refactoring techniques like Substitute algorithm (Strategy Pattern) can be applied. Similarly, if the duplicated code is only visibly similar, but isn’t quite identical, Form Template Method technique (Template Method Pattern) can be applied.

Lazy Class

There could exist classes in your code which has become obsolete or the functionalities are near-useless, you could employee techniques like Inline Class to remove it.

Maintaining code is expensive and any unused code should be carefully eliminated.

Data Class

Classes are identified by two factors – Properties and Behaviours.  If you stumble upon a class that has only properties, then it is a good candidate for Data Class code smell.

 The first step you need to take is to review the client code that uses the class, and review if the functionality could exist in the Data Class itself. You could also review if the conventional collection such as Arrays could be used to store the Data in the class, thus eliminating the need for class.

Dead Code

Dead Code is often resultant of changing requirements/specifications, and a development team that was unsure/lazy to clean up the code. A good IDE such as the Visual Studio can aid in detecting such code and can be eliminated.

Remember, it doesn’t need to be a method, the dead code could also be a unused parameter, which can be eliminated using the Remove Parameter refactoring technique.

Speculative Generality

How often have we been guilty of creating functionality in anticipation of future requirements, but ended up never having to use them ? It is a common trait in teams, which is highly infectious and needs to be avoided. XP Principles like YAGNI, KISS can be highly useful in this regard in educating the team to avoid such pitfalls.

Refactoring techniques such as Collapse Hierarchy an Inline class can be used for eliminating such code.

Code Smells : Change Preventers

If you ever have been in a situation when you need to make change in one place, but had to make changes in many places too, then Change Preventers is a code smell you should be vary off. Change Preventers is result of poor structuring of code and can be broadly categorized into 3.

Divergent Change

Symptoms of Divergent Change are obvious when you have to make changes in several unrelated methods when you need to make a single change in a class. The solution for the code smell lies in splitting up the class. We could segregate the different behaviors of the class using Extract Class or in scenarios where, different classes have same behavior, we could employ Extract SubClass or Extract SuperClass whichever seem appropriate for the particular scenario.

Shotgun Surgery

Another closely related Change Preventer is known as Shotgun Surgery, which is almost opposite to Divergent Change. With Divergent Change, many changes were required to a Single Class, while with Shotgun Surgery, a single change is made to several classes. This is a resultant of a single responsibility split among many classes and can happen due to ‘over-refactoring’.

The resolution lies in bringing together all associated methods to form a single class that has the responsibility rather than distributing it over multiple classes. Refactoring techniques like Move Method and Move Field can be employed for the purpose.

Parrallel Inheritance Hierarchy

If you find your self in situations where you intend to create a derived class, but also end up having to create sub classes for another class, then Parallel Inheritance Hierarchy. This eventually would result in making changes harder as related changes would be spread across the codebase.

To remove such duplication, we would need to employ refactoring techniques like Move Method and Move Field after creating a reference of the dependent class in your original class. Once all the methods and fields has been moved, you can get rid of the reference and duplicate class.

Code Smells : Object Oriented Abusers

While a well implemented Objected Oriented code is a hallmark of a good program, a poor implementation of the same could  lead to utter chaos as the project progresses. Object Oriented Abusers are a particular genre of Code Smells which refers to incorrect or incomplete implementation of Object Oriented Concepts.

Switch Statements

Switch Statements are something we are used to, it makes writing code faster. But does it make it easier to maintain ? Not really, in fact, switch is almost similar to if-else conditions, just that it is slightly more readable. Another issue with the Switch Statements (or for that matter the if-else conditions) is that it tend to mix logic and data together, which isn’t exactly the best way to do it. We would ideally want to separate the two, so that we can change them independently without interfering with each other. Each time the developer needs to change the logic or the data, he has a mess in his hands. Furthermore it is a violation of Open Closed Principle each time the developer needs to modify the cases to accommodate a new condition (or remove a condition).

The most frequent approach to solve this problem is known as Replace Conditional With Polymorphism, typically done using the Strategy Pattern. Variants of same, namely, Replace Type Code with SubClasses and Replace Type Code with State/Strategy can also be used. There could be situation where there aren’t too many conditions and all of them call the same method with different parameter. In this scenario, polymorphism just might be an overkill and better approach would be a Replace Parameter with Explicit Methods.

A special case that need consideration would be if one of the conditional options is null, wherein you could introduce the Null Pattern or Introduce Null Object.

Temporary Fields

There are times the developer decides to introduce Fields in the Class which are used only by one Method, instead of passing it as method parameter to avoid Long Parameter List. These fields sit idle in the class at all time, except when the particular method is used.

 

The way to tackle this particular type of code smell is using a variant of Replace Method with Method Object, ie, moving the entire method and its related field to a separate class. It would make your code much cleaner.

 

Refused Bequest

Another form of object oriented abuse, particularly inheritance, can be seen in derived classes, which doesn’t use most of the functionalities of the SuperClass. In fact, the SuperClass and SubClass is quite different form each other. These inevitably violates the Liskov Substition Principle. Consider the following example
public class BaseClass
{

public virtual void MethodA()
{
 Console.WriteLine("Do Something Usefull");
}

}

public class SubClass:BaseClass
{
public override void MethodA()
{
Console.WriteLine("Do Something More Usefull");
}
}
The above code is an example of Refused Bequest. The Child Class ignores the functionalities of Base Class (Refuses to implement SuperClass behavior) and Overrides it. Furthermore, This will also violate the Liskov Substitution Principle as the Inherited class cannot replace the BaseClass in a code without affecting the functionality.

 

The solution for this particular Code Smell lies on two different approaches, depending on the need of code. There might be a scenario where the Inheritance makes sense to a certain extend, despite symptoms of Refused Bequest, for example, it applies only to limited methods and data fields of super class. In such a scenario, Push Down Method  would be one of the refactoring technique you could opt for. Push Down Method recommends to move the method completely to the Sub Class as none of the sub classes uses the functionality provided by the Base or Super Class. The solution with Push Down Method applied would look similar to code below.
public class BaseClass
{

// Other used methods
}

public class SubClass:BaseClass
{
public override void Method()
{
Console.WriteLine("Do Something More Usefull");
}
}
A closely related technique would be Extract Super Class.

 

In certain other scenarios, the inheritance might seems purely superficial and the best approach would be to get rid of inheritance. That’s where Replace Inheritance with Delegation comes into picture.

 

Replace Inheritance with Delegation as the names suggests opposes the usage of inheritance when it seems like it is unwanted.  This approach suggests us to use an instance of the BaseClass and delegate the functionality to the instance, especially in scenarios when the Sub Class doesn’t require all the methods and data of the Base Class.

 

Alternative Classes with Different Interface

 The last type of Object Oriented Abusers we would be inspecting is known as Alternative Classes with Different Interfaces, which hints at a situation wherein two different classes seems to perform similar responsibilities, but have different interfaces (different method names, signature etc).

 

Such situation arises mostly when there is lack of communication between team members who end up developing similar classes, or when the developer fails to check the available classes.

 

The ideal approach towards treating this particular variant of Code Smell is by Renaming Methods or Moving Methods so that both classes end up implement the same interface. There would be other times when only a part of the classes are similar, leading to usage of Extract SuperClass.

 

Code Smells : Bloaters (Primitive Obsession, Long Parameter List, Data Clumps)

Primitive Obsession

If there is a code smell I often misses, then it has to be Primitive Obsession. Quite honestly, I have been guilty of falling for Primitive Obsession more than once. This particular type of Code Smell refers to the tendency of Developers to use primitive types instead of small objects for stimulating certain fields.  For example, one of the most common form of primitive obsession is usage of strings to represent Phone Numbers or Zip Codes.

 

Clearly, Phone Numbers and Zip Codes have their own formats and having primitive string to represent it would mean, you need to do additional checks (mostly repeated preconditions) in every method which utilizes it to ensure they hold the integrity needed by the stimulated object.  Instead, the Users could Replace Data Value With Object along with the associated behavior ( and validation checks).

 

Another possible technique associate itself when primitive fields are used in Method parameters. There might be cases when same group of parameters are used by more than one methods. Let’s consider the following code for example.
public IList GetBirthdays(DateTime startDate, DateTime endDate);
public IList GetAnniversaries(DateTime startDate, DateTime endDate);
public IList GetOtherLandmarks(DateTime startDate, DateTime endDate);
As seen in the above code,  two date time parameters, namely StartDate and EndDate are repeatedly used by 3 different methods. You could introduce a object here replacing the repeated parameters. The above code could be rewritten as following

 

public IList GetBirthdays(DateRange Dates);
public IList GetAnniversaries(DateRange Dates );
public IList GetOtherLandmarks(DateRange Dates );

 

This technique of Refactoring is known as Introduce Parameter Object as observed by Martin Fowler. Another technique related to primitive parameters being used as Method Parameters is called Preserve Whole Object. Consider the following code.

 

var employeeID = employeeInstance.GetEmployeeID();
var employeeDepartment = employeeInstance.GetDepartment();
var employeePaymentDetails = payment.GetPaymentDetails(employeeID, employeeDepartment);

 

Instead of calculating individual fields like employeeID and employeeDepartment, and then passing it to the GetPaymentDetails method, you could pass the employeeInstance object instead. This works in treating Long Methods  as  well.

 

The advantages of staying away from the Primitive Obsession include increased flexibility of code and well organized code. You have centralized place to maintain operations and conditions related to the particular data type. Also, it becomes easier for a future developer who doesn’t have to spend time guessing the limits/conditions related a particular data type.

 

Long Parameter List

How often have we ended up working with Methods which require more than 5 parameters ? Even harder would be existing code which utilizes a method which has many parameters. Remember, developers spend a better part of their development time reading existing code rather than creating a new one. So the implication of not writing easily readable code is quite large.  At least with .Net we do have Named Parameters which increases the readability a bit but at the end of the day, a method with 3 or 4 parameter is always easier to work with than one with 5-8 parameters. Ideally, anything more than 3 parameters for a method is a good candidate for refactoring.

 

So what are the effective refactoring techniques that could treat this particular kind of Code Smells.  We  have already discussed two earlier in this very blog post – Introduce Parameter Object and Preserve Whole Object. These two methods are effective in reducing the number of parameters used in a method.

 

A third technique, known as Replace Parameter with Method Call involves something similar. Consider the following code.

 

var employeeID = employeeInstance.GetEmployeeID();
var employeeDepartment = employeeInstance.GetDepartment();
var employeePaymentDetails = payment.GetPaymentDetails(employeeID, employeeDepartment);

 

Instead of the above code, where the daysWorked is calculated and passed to CalculateIncentive Method, we could have the CalculateIncentive method do the call to GetDaysWorked within itself, rather than being send as a Parameter. This would get rid of the parameter and simplify the method call.

 

Data Clumps

Another common bloater found in code is known as Data Clumps. Martin Fowler has coined the term as follows

 

“Whenever two or three values are gathered together, turn them into a $%#$%#^ object” – Martin Fowler

 

At times, you see a set of variables hanging around in groups in different part of the code. These are potential to be made into an object of their own.  If the group of variables are instance variable in class, then Extract Class is a good technique to apply. There might be other scenarios where the group of variables are as parameters of method, which gives us the option to employ Introduce Parameter Object and Preserve Whole Object.

 

Treating Data Clumps goes a long way in organizing code in a better way.

Code Smells : Bloaters (Long Methods, Long Class)

Code Bloats or Code Bloaters are probably the most common signs of code smells you can see in any branch of code. In fact, eradicating bloaters act as the first step towards refactoring your code. Bloaters are nothing but classes or methods that have grown excessively over a long time marking it difficult to work with. Typically, bloaters doesn’t happen right away, but is accumulate long term as your code base grows.

In this blog post, we will be discussing two of the most common bloaters, Long Method and Long Class

Long Methods

It is recommended that any method which is longer than 10 lines of code is a good candidate for refactoring. One could Extract Code Fragments (that does a specific task and relates to each other than any other piece of code in the method) that can be grouped together to a new Method (Extract Method), even if the new method is hardly couple of lines. It could also help in reducing code duplication and isolating independent parts of code.

Another refactoring technique that could be applied for Long Methods is Separating Query from Modifiers . It would be better to segregate the code fetching data from that modifies/mutates a state. The crux of this technique is based on the Command Query Seperation (CQS) Principle. CQS recommends dividing an object’s method into two broad categories, Queries (Returns a result, but doesn’t change any observable state) and Commands (changes observable state but doesn’t return a value). An observable state of an object is a state which can be accessed by the client code. However, care should be taken to avoid race condition that comes along the CQS approach in a multi-threaded application.

There might be certain scenario’s as well where the local variables are so intertwined with the code that it becomes difficult to extract method. In such scenarios, it might be a good idea to Replace Method with a Method Object, or in other words separate the Long Method into a Separate class, where the local variables of the class becomes Fields variables of the new class. Then it becomes much easier to split the method to smaller chunks of code.

Conditional Operators such as If-else or Switch Cases is another place where you could cut down the Method. You could either opt for refactoring techniques such as Decompose Conditional or use Strategy Pattern. Usage of Strategy Pattern also helps in bringing down the Cyclomatic Complexity of the Method.

Large Class

Large Class, similar to Long Methods are another undesired quality accumulated over time. Developers find it more convenient to add methods to existing classes than creating a new one. This results in Classes which cluttered with large number of methods and fields.

It is always better to split up a large class (Extract Class), especially if it has more than one responsibility. This ensures the class aligns with Single Responsibility Principle. There might be other cases where , there might be a class which utilizes a subset of its feature only in certain special scenarios. For Example consider the following code.

public class Person
{

void PublishMarks()
{
if(PersonType == PersonType.Student)
{
// Do Something
}
}

void PublishSalary()
{
if(PersonType == PersonType.Faculty)
{
// Do Something
}
}

}

In the above class, despite being related to the Person Class and sharing many other features with it, methods PublishMarks and PublishSalary are used only when the PersonType is Student or Faculty respectively. In this scenario, separating an entirely new Class for Student and Faculty isn’t quite recommended. Instead, we could take advantages of Inheritence and creates subclasses of Person Class, separating the Student and Faculty functionalities to SubClass. This technique is refactoring is known as Extracting SubClass.

Another scenario that is worth mentioning is cases when Domain (Business) Logic and Data gets embedded in the UI (Presentation) class. Problem with this approach is that you might end up with lot of duplicate code as the same domain objects might be required at other places in Presentation Classes. The refactoring technique, Duplicate Observed Data, recommends to remove Domain Data to separate classes and use an Observer pattern to synchronize the classes.

In the next of this series, we will look into few more Bloaters, including Primitive Obsession, Long Parameter List and Data Clumps.

References :
https://refactoring.com/catalog/
https://sourcemaking.com/

Patterns,Principles and Programming.

Have been thinking about putting together a collection of Jump Start Tutorials on some core concepts on Programming, finally getting everything together

Design Patterns

Design Princples

Code Smells and Refactoring Techniques