This content originally appeared on DEV Community and was authored by Jan Peterka
I have a lot of fun lately with diving into old and new parts of our ~15yo app at $DAYJOB, and trying to improve it bits by bits.
One of the issues, as many older apps may have, are "god objects" - objects with so much logic inside them, they are mostly impossible to understand and maintain.
It can happen to any object - you start with something small and concrete, possibly adhering to Single responsibility principle even. But then, you need to add just one more method to the object, and one more after that, some scopes, validations, callbacks.
And when new developer joins the project, they see that this object has so many to do, and so many lines, it won't hurt to just add another.
And then you see their pull request, think "oh, we should someday clean this, it's terrible", but that's not the juniors responsibility, so you just sigh and approve this new addition, solidifying this monstrosity.
I was once the junior, having no worries about the code design, just doing what I saw the more experienced devs doing. So I contributed my share to this mess.
Now, I'm bit more experienced, bit more aware of consequences, and bit less careless about this kind of code.
Now I feel it's my responsibility to visit this Augean Stables, and emerge, if not victorious, than at least proud of some improvements I'm able to do.
Luckily, I also really enjoy it (well, most of the time).
So how do I start?
Finding topics
I try to find out bundles of behavior and data that belong together - our object surely contains more than one responsibility (or topic) now, lets see what is there.
For example, we have object Job in our app. It does a lot of things (and have more then one role, which complicates things a bit), but let's say that it mostly describes demands users create in our application.
Demand has and does a lot of things - it can be commented and bided upon, can be reviewed, rated, priced, approved, verified, categorized, located, and many more things.
Let's say we have something like this:
# app/models/job.rb
class Job < ApplicationRecord
# ...a lot of code
scope :not_rated, -> { where(rating: nil) }
# ...different code
def add_rating(rating)
# does some logic
send_notification_about_updated_rating
end
# ...more code
def send_notification_about_updated_rating
# does some notifying
end
# ...it never ends...
end
If you have any experience with Rails, you probably see where I'm heading.
Yes, I will heavily lean on Concerns (and ruby modules in general).
Note on cautiosness: Now, there are people who don't like Concerns, as they can lead you to the same god object as I described, just hidden in multiple files. And I understand their concerns, and yes - that can happen and is not great. See Jason Swett's article in resources at the end of this article to read more.
However, whether you like them or not, and whether you want them in your app, they can be a great tool for what I'm trying to achieve here. Bear with me.
Extracting topics to Concerns
Once I found some topics in my object, I will create a new concern for them, and carefully find anything related to move there:
# app/models/concerns/job/rateable.rb
module Job::Rateable
extend ActiveSupport::Concern
included do
scope :not_rated, -> { where(rating: nil) }
end
def add_rating(rating)
# does some logic
send_notification_about_updated_rating
end
def send_notification_about_updated_rating
# does some notifying
end
end
# app/models/job.rb
class Job < ApplicationRecord
include Job::Rateable # I can also omit the `Job::` part, but I like it.
# ...a lot of code
# ...different code
# ...more code
# ...it never ends...
end
Note on testing: Even though moving things to concern seems pretty safe, I got failing tests after this many times (and sometimes errors in production, when tests were missing or incomplete). So, first step should be to check for tests, and have a general idea about what is tested. Also, if there are no tests, it is useful to add some.
If successful, I now have one file with all related code to this topic (e.g. Job can be reviewed).
This has immense benefit in itself - hopefully now I can get proper idea about what's used, what behavior is expected. I don't need to understand all logic, just overall feel.
Improving code
Now I can start to restructure this a bit. Possibly there are some methods that can be marked as private, making it clearer what is the public interface of our Job in the topic of reviewing.
Note on private methods in Ruby: There's some discussion on how to use private in ruby. Marking method private doesn't really stop you from using it - you can call it using
send, for example.
I do use it mostly to make it clear that it's just internal details, not public interface. It makes it clearer to me what I'm dealing with.
In our example, notifying about rating is probably only done on methods in our concern, so we can mark it private:
# app/models/concerns/job/rateable.rb
module Job::Rateable
extend ActiveSupport::Concern
included do
scope :not_rated, -> { where(rating: nil) }
end
def add_rating(rating)
# does some logic
send_notification_about_updated_rating
end
private
def send_notification_about_updated_rating
# does some notifying
end
end
Note on testing: Now I'm doing real changes to my code, and I'm very happy to have tests.
Great, my idea about this area of code is getting better and better - I have everything in one place, I know what is public interface and what are internals, maybe I even found some old code that is no longer used in our app (git log -p -S is my good friend here), which is a nice bonus.
But, we still have a model with a lot of code, lot of responsibility on them.
Luckily, we have all we need to move one step further, and create truly well designed code.
Once again, it's just PORO
As we have all the logic in one place, we might be able to extract it to one or more POROs (plain old ruby objects):
# app/models/job_rating.rb
class JobRating
attr_reader :job
def initialize(job)
@job = job
end
def add_rating(rating)
# does some logic
send_notification_about_updated_rating
end
private
def send_notification_about_updated_rating
# does some notifying
end
end
We now have self-contained class, with one responsibility - adding rating (in real world, it probably also includes updating rating, deleting rating and so on. but still.). Now, finally, job has no bussiness in managing its ratings - that's job of this object!
But wait! Do we have to find all places where we call job.add_rating and rewrite it to JobRating.new(job).add_rating? No!
First, that would be ugly as hell.
Second, that's a lot of work I'm not willing to do.
Third, now all these places would need to know that there's JobRating class, ugh.
Note on preferences: Maybe you are happy to stop here, and get rid of the Concern that helped us get here. That's okay too! Using concern the way I do in next part has its drawbacks - it's less clear where the logic resides and you have to follow the trail of delegation, also it makes it cosy to just stick more and more to the concern, getting the same issues we had before. I like structuring my code this way, but you don't have to!
Delegating responsibilities
No, there's better way - we leverage Concerns again:
# app/models/concerns/job/rateable.rb
module Job::Rateable
extend ActiveSupport::Concern
delegate :add_rating, to: :rating
included do
scope :not_rated, -> { where(rating: nil) }
end
def rating
@rating ||= JobRating.new(self)
end
end
Now, we don't have to change anything anywhere! Rest of our codebase doesn't care - it just wants to add rating to job! Now job accepts this request, and delegates it to rating object.
Note on inspiration: This pattern is not my discovery, but I'm now unable to find the article I read introducing it. If you know where it came from, let me know and I will add it to resources.
Here I showed how using Concern as a stepping stone helps me to make messy code cleaner and better separated.
What do you think? Do you also use this approach, or do you have other tricks to clean this sort of code?
And what are your opinions on (over)using Concerns? Let me know!
Resources and more reading
Here's some reading about Concerns, if you want a bit more context:
Concerns in Rails: Everything You Need to Know (Akshay Khot @ Write Software Well)
When used intelligently, Rails concerns are great (Jason Swett)
Vanilla Rails is plenty (Jorge Manrubia)
This content originally appeared on DEV Community and was authored by Jan Peterka
Jan Peterka | Sciencx (2025-03-17T15:06:00+00:00) Concerning God objects in Rails legacy app. Retrieved from https://www.scien.cx/2025/03/17/concerning-god-objects-in-rails-legacy-app/
Please log in to upload a file.
There are no updates yet.
Click the Upload button above to add an update.