Refactoring Day 27 : Remove God Classes

Often with legacy code bases I will often come across classes that are clear SRP violations. Often these classes will be suffixed with either “Utils” or “Manager”. Sometimes they don’t have this indication and are just classes with multiple grouped pieces of functionality. Another good indicator of a God class is methods grouped together with using statements or comments into seperate roles that this one class is performing.

Over time, these classes become a dumping ground for a method that someone doesn’t have time/want to put in the proper class. The refactoring for situations like these is to break apart the methods into distinct classes that are responsible for specific roles.

   1: public class CustomerService

   2: {

   3:     public decimal CalculateOrderDiscount(IEnumerable<Product> products, Customer customer)

   4:     {

   5:         // do work

   6:     }

   7:  

   8:     public bool CustomerIsValid(Customer customer, Order order)

   9:     {

  10:         // do work

  11:     }

  12:  

  13:     public IEnumerable<string> GatherOrderErrors(IEnumerable<Product> products, Customer customer)

  14:     {

  15:         // do work

  16:     }

  17:  

  18:     public void Register(Customer customer)

  19:     {

  20:         // do work

  21:     }

  22:  

  23:     public void ForgotPassword(Customer customer)

  24:     {

  25:         // do work

  26:     }

  27: }

The refactoring for this is very straight forward. Simply take the related methods and place them in specific classes that match their responsibility. This makes them much finer grained and defined in what they do and make future maintenance much easier. Here is the end result of splitting up the methods above into two distinct classes.

   1: public class CustomerOrderService

   2: {

   3:     public decimal CalculateOrderDiscount(IEnumerable<Product> products, Customer customer)

   4:     {

   5:         // do work

   6:     }

   7:  

   8:     public bool CustomerIsValid(Customer customer, Order order)

   9:     {

  10:         // do work

  11:     }

  12:  

  13:     public IEnumerable<string> GatherOrderErrors(IEnumerable<Product> products, Customer customer)

  14:     {

  15:         // do work

  16:     }

  17: }

  18:  

  19: public class CustomerRegistrationService

  20: {

  21:  

  22:     public void Register(Customer customer)

  23:     {

  24:         // do work

  25:     }

  26:  

  27:     public void ForgotPassword(Customer customer)

  28:     {

  29:         // do work

  30:     }

  31: }

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.
  • Roxin

    Would you also split CustomerOrderService into another 2 classes ? OrderCalculators , CustomerOrderValidation (for the first 2 methods ) – would this be overkill ?

  • http://www.davetheninja.net Dave the Ninja

    @Roxin

    I would split it down further as you mentioned as the calculator and validation IMO have completely separate responsibilities.

    David

  • Lyronne

    In this example the refactoring results look similar to a Service Layer (Fowler, POEAA, Service Layer). The difference between this and Service Layer is that service layer delegates to a domain model. I know this example is really about SRP, but can we afford to ignore the possibility of these classes not existing at all. The opportunity exists to connect these operations to the objects they operate on, by enriching Customer, Order, Product etc. with the appropriate operations in these classes, the classes become closely mapped to the domain language and IMO these classes are ambassadors for SRP.

  • Dave Two

    I’m more interested in findging out what calls these service classes. If this is a “controller” type class, do we now need to maintain two references, or pass two references around instead of one? Or do we bundle this under a “ServiceManager” that maintains references to all of these sub-services?