Monkeypatching for cleaner code

Recently, after reading Object on Rails, I started thinking and experimenting with various ideas of making Rails applications code cleaner. Here’s one of these ideas.

Let’s imagine we have two model classes, connected with a has-many/belongs-to relation, e.g.:

# file user.rb
class User < ActiveRecord::Base
  has_many :posts, dependent: :destroy
  # ...
  # rest of the user stuff
  # ...
end
# file post.rb
class Post < ActiveRecord::Base
  belongs_to :author, class: "User"
end

The above code is probably how most of Rails programmers would go about implementing a “user has many posts/post belongs to its author” scenario. It’s the tutorials-approved way. But when you look at it from a architectonic point of view, you have just created a circular dependency.

Rails developers sneer at dependencies, since Rails handles finding and loading automatically. Probably half of us never had to use require in our code. Of course, circular dependency in Rails is nowhere as bad as in C#, C++ or Java. But still, if you don’t pay attention to this problem, your models become a bunch of tightly coupled, entangled classes that cannot be used and tested without all of their friends. Modularizing application in this case becomes a really difficult task.

OK, so how do we deal with this problem? One option is to remove the has_many :posts snippet from User class. Doing this, however, we lose all of the goodies that has_many provides: navigating to user’s posts, filtering them, building new objects that are automatically connected to the user, automatic removal of all posts together with their author etc.

But does this functionality really belong to User class? To me, this is a violation of Single Responsibility Principle. Class should only have one reason for change. Changing a (presumably finished) User class in order to add a Post class is an example of Shotgun surgery anti-pattern. (In fact, we need to change the User class every time we add any class that has some relation to User.)

Let’s look at it this way: can a user exist without any posts? Sure, every newly created user has no posts. There might be users that will never create any posts. On the other hand, can a post exist without an author? A blog post always has an author, unless it’s a random sequence of letters from a random number generator. The belongs_to :author relation defines a strong dependence from post to user. Good practice says dependencies between layers should always go only in one direction. We should try to achieve this at class level, too, as much as possible.

Let’s try a thought experiment. How about injecting the has_many :posts relation into User class only when the Post class is loaded? In Ruby, it’s easily done

# file user.rb
class User < ActiveRecord::Base
  # ...
  # the user stuff, nothing about posts
  # ...
end
# file post.rb
class Post < ActiveRecord::Base
  belongs_to :author, class: "User"
end

class User < ActiveRecord::Base
  has_many :posts, dependent: :destroy
end

Now, is this code cleaner? Until we reference the Post class in our code, the User does not “know” anything about posts. In fact, we can’t even use user.posts method — this results in a NoMethodError. In a Rails application that should not be a problem, the Post class will be mentioned sonner or later, injecting the has_many :posts relation.

What are the pros & cons of this approach? On the pros side, the User class is free of any reference of the Post class. We are not tempted to sprinkle User class with post_count or published_posts methods, that always seem like useful shortcuts at first, but in the long run make our code unmaintainable. All the code related to posts is now in the post.rb file, even if technically we still add the has_many :posts to User class.

On the cons side, it’s harder to find everything that constitutes the User class at runtime. This class is reopened when other files are loaded and new methods are defined on it. In the end, its interface is as cluttered as it was in the beginning. There’s also a possibility that we may somehow stumble on some missing relation, because some of the files have not been loaded yet (to be honest, I didn’t test the idea too extensively and I don’t know how likely is that problem, but my feeling is that it’s not).


Leave a comment