Replace multidimensional if condition with double dispatch.

If you have a series of if conditions that vary in two dimensions it may be possible to replace the conditionals with double dispatch.


public class passenger
{
public Vehicle vehicle;
public string type;
public void travel()
{
if (vehicle.Type == "Car")
{
if (this.Type = "Rich")
{
RideInLimo();
}
else if (Passenger.Type = "Poor")
{
Drive();
}

else if (vehicle.Type = "Airplane")
{
if (this.Type = "Rich")
FlyInPrivateJet();
else if (Passenger.Type = "Poor")
{
FlyCoach();
}

}
}
}
}

In the above example the first refactoring that I would like to apply is Replace Type Code with Class. However the way a person travels does not only depend on the type of vehicle he uses but also the type of person they are as well. This problem can be solved by passenger passing itself as a parameter to the vehicle class. Then the vehicle class can use method overloading to create to versions of the travel method for each type of person.

Here is the refactored code.



public class Passenger
{
public Vehicle vehicle;
public void Travel()
{
vehicle.Travel(this);
}


}

public class Car : vehicle
{
public void Travel(RichPersion Passenger)
{
RideInLimo();
}
public void Travel(PoorPersion Passenger)
{
Drive();
}

}


public class Plane : vehicle
{
public void Travel(RichPerson Passenger)
{
FlyInPrivateJet();
}

public void Travel(PoorPerson Passenger)
{
FlyCoach();
}

}
One drawback of using this approach is that it violates the Open Close Principle because you will have to add a new version of the travel method on the vehicle object for each new person type that you add to your application.

Move Foreign Method to Server Class

When I am working with code that is not object oriented I often see a bunch of objects that just contain data and not much behavior. These methods are often passed to helper classes that perform the real work.

Class MyHelperClass
{
public static void DoSomething(MyObject thing)
{
Dothis(thing.This);
Dothat(thing.That);
DoTheother(thing.Other):

}
}

Whenever I see static helper functions I look at the parameter list for objects that would be a better home for the function.

This specific instance of the Move Method refactoring is kind of the reverse of the Foreign Method refactoring. So my name for this refactoring is "Move Foreign Method to Server Class"

Fluent Constructors

Something in the following code just does not look right to me.

Employee employee = new Employee();
employee.EmployeeNumber ="1565407"
employee.Name = "Nick"
employee.LastName = "Markovic"
employee.JobFunction = CreateJobFunction("Tester")


The Employee Constructor just tells me nothing on how to use the object.

It does not tell which properties are required and which are not.
In this example EmployeeNumber, Name and LastName were all required but JobFunction was not. To make things worse the employee object has about 20 other properties on it. I came across this problem while I was writing unit tests for legacy code and needed to create a simple employee to pass as a parameter. I had to go digging through the internals of employee to determine which fields needed to be set.
However if the creator of this class had crated a constructor that looked something like Employee(number,firstName,LastName). The interface would be much more fluent and would create much easier to understand code.

Strings - The only data type you will ever need.

I have seen lots of code that looks like the following

Person candidate = CreatePerson()
string hireDate= "02/12/2005"
string departmentId = "12";
HirePerson(person,hireDate,departmentId)

I often see methods that take strings as parameters when they are dealing with values that should be stored in more expressive data types. Usually this happens with values that get pulled out of text boxes on the UI. These parameters start getting pasted all over the place and only end up getting cast to a more specific data type when some special functionality like date comparison is needed.I have also seen this happen with id values. They often get converted back and forth from int to string as they get passed around the application. I guess this code smell is kind of like primitive obsession taken to the extreme.