Tuesday, March 24, 2009

Following SRP on grounds

Single responsibility principle is probably the simplest OO coding principle. It says "A class should have only one responsibility and thereby, only one reason to change." Isn't it very straightforward ?
But when I look at the code of my application, I see that very few classes in my app follows this principles. Why is that ? The principle is very simple and so it should be simple to implement as well. Its relatively easy to make sure that our class doesn't have any method that doesn't belong to the job of class. I think the difficult part is to know when we are adding additional responsibilities to the class. We do incremental development, start with only what is required and then add up behavior in methods and methods to class as required by the failing test. During this process, we somehow manage to sneak in the logic that is not the part of class's responsibility.
For example, we have mapper classes in our code which maps domain object to data contracts. The job of mapper classes is just to copy the fields from domain objects to contact. The mapper should only do this job of mapping. Consider the following example. In this, Booking object is loaded from the database. This class will map Booking to BookingData object.

public class BookingMapper{

public BookingData Map(Booking booking)
{
BookingData bookingData = new BookingData();
bookingData. Id = booking.Id;
// Other fields
bookingData.BookingFee = CalculateBookingFee (booking.StationCode, booking.TotalCost);

return bookingData;
}

private decimal CalculateBookingFee (StationCode stationCode, decimal totalCost){
// some complex logic to calculate booking fee based on station code and total cost,
// may be, the method will read some values from database.
}
}

The class BookingMapper has only one Map() method. From outside the class, it looks like the class has only one responsibility. However, apart from mapping the data, the class is also calculating the booking fee. It has 2 jobs - to map the data and to calculate the value of booking fee.

The mapper class should do only mapping and we have to move the responsibility of calculating booking fee to other class. If the fee calculation depends only on data present in booking object (like station code and total cost), then the obvious place for CalculateBookingFee method is Booking class itself. We can have a read-only property on Booking class called BookingFee which will calculate the fee.If the calculation of fee involves reading the values from database and/or from some other service, then we don't want our domain object to access services or database. In this case, we can calculate the booking fee (and any other required values) before invoking the mapper and pass it to Map() method. Either way, the fee calculation logic will move out of BookingMapper.

No comments: