'Classify' Your Codebase
Photo by CHUTTERSNAP on Unsplash
Most of the software engineers have a strong (theoratical) grip on OOPs concepts right from the college days but few still struggle to apply them while working. There was a time in my career when I realized that my code is doing everything it is 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 is counter-productive.
Few years ago, I worked on upgrading Rails major version of a monolith project (an app that powers most of 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, but 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 (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 clearly 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 ecommerce 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
There are multiple questions that come to my mind when I see such a code, but the fundamental one is - Why did we keep this method (and all it’s 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 clearly 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 rollback the previous steps if any step fails. And if we want to build such a process, would we want to put those methods also in the Product
class? (The above code is influenced from Fat models Thin Controllers philosophy).
The Answer
What do you think? Should you take a course on Design Patterns in order to refactor this? Few months ago I read a book on the same, and I’m sure that there are a lot of improvements we can do 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 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 discount, 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 out, 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 ok. Otherwise you might want to refactor your classes. Happy coding!