Friday, November 29, 2013

Rule #2: Code Duplication is Bad!

Every now and again I encounter a situation where I need to use functionality from one class and apply it in another one. The 2 classes are not aware of each other, but still need to use the same functionality.
For example:

This class handles offers in an e-commerce application:

Class OffersResource {
    public List<Offer> getOffersForUser(String userName) {
        User user = getUserDetails(userName);
        // do something with the user and query offers
        return offers;     } 

    private User getUserDetails(String userName) {
        // query DB
        return User;
    }
}

Now suppose we want to send a notification for the user. We will have a class that handles notifications:

Class Notifier {
     public void sendNotification(String userName) {
         // we need to get the user's details and email/phone number
         // once we have the user's details, we can send notification
     } 
}

One option is to copy-paste the getUserDetails(String userName) method from the OffersResource class. 

This is bad design! Now what happens if we need to change the getUserDetails method? Suppose that we want to add some kind of validation or some business logic, we'll have to do it twice. Or if we find a bug, most likely that it will get fixed only in one instance.
This is a maintenance nightmare - maintaining several copies of the same code. In my example there is only one duplication of the code, but it could be more - and that will multiply our agony :(...

The proper way to do it is to refactor the getUserDetails method into a 3rd class - let's call it UserService. Both classes will have an instance of that class:

Class OffersResource {
    private UserService userService;
    public List<Offer> getOffersForUser(String userName) {
        User user = userService.getUserDetails(userName);
        // do something with the user and query offers
        return offers;
    } 

    private User getUserDetails(String userName) {
        // query DB
        return User;
    }
}

and: 

Class Notifier {
    private UserService userService;
    public void sendNotification(String userName) {
        User user = userService.getUserDetails(userName);
        // send notification     } 
}


 By doing so, we will gain the following:
  • Loose coupling. Both the OffersResource and the Notifier classes need not to know how to fetch users, and only have to deal with their business logic.
  • Single point of maintenance. Whenever we need to change our method or fix a bug in it - we do it only once.
  • Cleaner, readable code. Suppose someone who is not familiar with your code needs to change the way Users are fetched from DB. The first place he will look is the UserService class. If we didn't have that class - he would have to look for it in all other classes, which will take longer.
Read more about the DRY principle.

 Happy coding!

No comments:

Post a Comment