Refactoring Day 14 : Break Responsibilities

When breaking apart responsibilities on a class this is enforcing Single Responsiblity Principle from SOLID. It’s an easy approach to apply this refactoring although it’s often disputed as what consitutes a “responsibility”. While I won’t be answering that here, I will show a clear cut example of a class that can be broken into several classes with specific responsibilities.

   1: public class Video
   2: {
   3:     public void PayFee(decimal fee)
   4:     {
   5:     }
   6:  
   7:     public void RentVideo(Video video, Customer customer)
   8:     {
   9:         customer.Videos.Add(video);
  10:     }
  11:  
  12:     public decimal CalculateBalance(Customer customer)
  13:     {
  14:         return customer.LateFees.Sum();
  15:     }
  16: }
  17:  
  18: public class Customer
  19: {
  20:     public IList<decimal> LateFees { get; set; }
  21:     public IList<Video> Videos { get; set; }
  22: }

As you can see here, the Video class has two responsibilities, once for handling video rentals, and another for managing how many rentals a customer has. We can break out the customer logic into it’s own class to help seperate the responsibilities.

   1: public class Video
   2: {
   3:     public void RentVideo(Video video, Customer customer)
   4:     {
   5:         customer.Videos.Add(video);
   6:     }
   7: }
   8:  
   9: public class Customer
  10: {
  11:     public IList<decimal> LateFees { get; set; }
  12:     public IList<Video> Videos { get; set; }
  13:  
  14:     public void PayFee(decimal fee)
  15:     {
  16:     }
  17:  
  18:     public decimal CalculateBalance(Customer customer)
  19:     {
  20:         return customer.LateFees.Sum();
  21:     }
  22: }

This is part of the 31 Days of Refactoring series. For a full list of Refactorings please see the original introductory post.

Related Articles:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Sean Chambers

I am a Senior software developer from Palm Coast, Florida. An advocate of Domain Driven Design, Behavior Driven Development, creator of FluentMigrator and community activist. I am married to my beautiful wife Erin and am the proud father of two wonderful children. I currently reside at ACI, a local insurance industry/mortgage software company that excels in creating solutions using Agile methodologies.
This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://www.clausjoergensen.dk Claus Jørgensen

    Since the Video.RentVideo method isn’t static, why would you need to pass the video argument along, opposite to doing it like this:

    public void RentVideo(Customer customer) { customer.Videos.Add(this); }

  • Brad

    Why is RentVideo in Video and accepting a video object?

    Wouldn’t it be better to either move RentVideo to the customer object and only accept a video object OR rename it to RentToCustomer (or something along those lines) and only accept a customer object? With the first option being the preference, as video shouldn’t have to deal with the customer objects internal collection.

  • Brad

    And why does CalculateBalance accept a customer object?

  • Robert

    It’s great you post these refactorings, and they mostly make sense. Although I would really appreciate you answering the above asked questions, because this way it makes things more confusing.

  • http://www.lostechies.com/members/schambers/default.aspx schambers

    @Robert the example is a bit contrived so the two other comment posters are partially correct. You can just pass this to the videos.add(this) method, although I don’t agree that you should move the RentVideo method to the Customer object. That violates single responsibility principle in the fact that now the customer is in charge of maintaining account balance AND renting videos which is a mix of responsiblities.

    I’m not sure how this makes it more confusing. It’s simply applying the single responsibility principle. When you start to get more than one reason for a class to change it has too many responsibilities and should be broken up into more distinct classes.

    Does that make sense?