'Classify' Your Codebase

Photo by CHUTTERSNAP on Unsplash

'Classify' Your Codebase

Most software engineers have a strong (theoretical) grip on OOP concepts right from their college days but few still struggle to apply them while working. There was a time in my career when I realized that my code was doing everything it was expected to do but I was unhappy with how it looked. I have written methods that were even 200-300 lines long, and classes 1000 lines long. But today if I see any such code, I call it unmaintainable and untestable. In this post, I want to share one thing that helped me transform my mindset, i.e. The power of small changes. It took me months to understand how this habit is more impactful because earlier I thought this was counter-productive.

A few years ago, I worked on upgrading the Rails major version of a monolith project (an app that powers most of the LocoNav website). Most of the changes here were pushed in one large pull request (100+ commits, 270+ file changes, 1300+ additions, 1600+ deletions, all by just me). Although it didn’t break anything related to code on production, I was quite nervous at the time of release. No one even reviewed this PR much because it was hard to review (as discussed in the first point here). Also, I found out that while raising large PRs worked for me, it did not scale for my team. Many such PRs sent by others caused production bugs.

We had a senior engineer who always advocated for small PRs and slowly I got influenced to follow the same. Being in a startup environment, we had to be fast enough in terms of delivery, so the main concern was - How can we break changes into small parts and still deliver fast? We started looking in the PRs on how we were organising our classes. We found out that we’re mixing a lot of responsibilities and not following SOLID principles at all (forget design principles!). You might think that I’m mixing basic OOPS with SOLID and other design principles (eg. prototype, builder or composite). But if you think carefully – it’s all about creating small classes that can talk to each other to complete a task. Let’s talk about a simple inheritance example that we studied during college days (the code sample is in Ruby but should be basic enough to understand):

class Animal
  def eat
  end

  def sleep
  end
end

class Dog < Animal
  def bark
    puts "BARK!"
  end
end

Here, we see that one class is doing one task (just defining a specific Animal, or adding more behaviour in subclass). Now let’s move to a real-world example of processing an order on an e-commerce website:

class Product < ApplicationRecord # Inheriting from ApplicationRecord means this is a model ('M' in MVC)

  def sell
    check_inventory
    apply_discount
    make_bill
    process_order
    send_to_dispatch_team
  end

  def check_inventory
    # logic here
  end

  def apply_discount
    # logic here
  end

  def make_bill
    # logic here
  end

  def process_order
    # logic here
  end

  def send_to_dispatch_team
    # logic here
  end
end

Multiple questions come to my mind when I see such a code, but the fundamental one is - Why did we keep this method (and all its associated methods) inside a model (or inside one class)? While this might look simple for once, it is doing a very complicated task. It’s visible that we might be calling other services or a payment system while processing an order, and hence any step can fail. There seems no defined way to roll back the previous steps if any step fails. If we want to build such a process, would we want to put those methods in the Product class? (The above code is influenced by Fat models Thin Controllers philosophy).

The Answer

What do you think? Should you take a course on Design Patterns in order to refactor this? A few months ago I read a book on the same, and I’m sure that there are a lot of improvements we can make before applying design patterns. Developers often shy away from doing this but the only solution to have a maintainable codebase is to break such classes.

Whenever you see a logical chunk in a class, that is not related to the rest of the code and can be taken out, please take that out. We often think that we can do all this later, but that time never comes till we see some serious issues with the code. Due to this delay, the methods inside the class become more coupled to each other and the effort of taking that out becomes much larger. The above is a very simple example where you could have a plain ruby class (PORO) called OrderProcessingService that has all the business logic for processing an order. And obviously, if needed, we’ll have more classes for performing other steps (checking inventory, applying discounts, etc.). It’s good to know about SOLID Principles and Design patterns, but at the end of the day you see - it’s all about distributing responsibilities, reducing coupling and creating more (small) classes. If you know how to apply this, you can apply design patterns as well. Otherwise, everything is theory!

So the next time you see yourself or a colleague writing methods in a class that should be separated, please make sure you do that. That will pay you in future 😀. Another way of knowing about your code’s health is to write test cases for it. If it’s easy to write test cases for a class without mocking a lot of things (except external services), then it’s okay. Otherwise, you might want to refactor your classes. Happy coding!