Welcome back to the second post in the Ruby on Rails Patterns and Anti-patterns series. In the last blog post, we went over what patterns and anti-patterns are in general. We also mentioned some of the most famous patterns and anti-patterns in the Rails world. In this blog post, we'll go through a couple of Rails model anti-patterns and patterns.
If you're struggling with models, this blog post is for you. We will quickly go through the process of putting your models on a diet and finish strongly with some things to avoid when writing migrations. Let's jump right in.
Fat Overweight Models
When developing a Rails application, whether it's a full-blown Rails website
or an API, people tend to store most of the logic in the model. In the
last blog post, we had an example of a Song class that did many things.
Keeping a lot of things in the model breaks the
Single Responsibility Principle (SRP).
Let's have a look.
1class Song < ApplicationRecord
2 belongs_to :album
3 belongs_to :artist
4 belongs_to :publisher
5
6 has_one :text
7 has_many :downloads
8
9 validates :artist_id, presence: true
10 validates :publisher_id, presence: true
11
12 after_update :alert_artist_followers
13 after_update :alert_publisher
14
15 def alert_artist_followers
16 return if unreleased?
17
18 artist.followers.each { |follower| follower.notify(self) }
19 end
20
21 def alert_publisher
22 PublisherMailer.song_email(publisher, self).deliver_now
23 end
24
25 def includes_profanities?
26 text.scan_for_profanities.any?
27 end
28
29 def user_downloaded?(user)
30 user.library.has_song?(self)
31 end
32
33 def find_published_from_artist_with_albums
34 ...
35 end
36
37 def find_published_with_albums
38 ...
39 end
40
41 def to_wav
42 ...
43 end
44
45 def to_mp3
46 ...
47 end
48
49 def to_flac
50 ...
51 end
52end
The problem with models like these is that they become a dumping ground for the different logic related to a song. Methods start piling up as they get added slowly, one by one over time.
I suggested splitting the code inside the model into smaller modules. But by doing that, you are simply moving code from one place to another. Nonetheless, moving code around allows you to organize code better and avoid obese models with reduced readability.
Some people even resort to using Rails concerns and find that the logic can be reused across models. I previously wrote about it and some people loved it, others didn't. Anyway, the story with concerns is similar to modules. You should be aware that you are just moving code to a module that can be included anywhere.
Another alternative is to create small classes and then call them whenever needed. For example, we can extract the song converting code to a separate class.
1class SongConverter
2 attr_reader :song
3
4 def initialize(song)
5 @song = song
6 end
7
8 def to_wav
9 ...
10 end
11
12 def to_mp3
13 ...
14 end
15
16 def to_flac
17 ...
18 end
19end
20
21class Song
22 ...
23
24 def converter
25 SongConverter.new(self)
26 end
27
28 ...
29endNow we have the SongConverter that has the purpose of converting songs to a
different format. It can have its own tests and future logic about converting.
And, if we want to convert a song to MP3, we can do the following:
1@song.converter.to_mp3To me, this looks a bit clearer than using a module or a concern. Maybe because I prefer to use composition over inheritance. I consider it more intuitive and readable. I suggest you review both cases before deciding which way to go. Or you can choose both if you want, nobody is stopping you.
SQL Pasta Parmesan
Who doesn't love some good pasta in real life? On the other hand, when it comes to code pasta, almost no one is a fan. And for good reasons. In Rails models, you can quickly turn your Active Record usage into spaghetti, swirling around all over the codebase. How do you avoid this?
There are a few ideas out there that seem to keep those long queries
from turning into lines of spaghetti. Let's first see how database-related
code can be everywhere. Let's go back to our Song model. Specifically, to when we
try to fetch something from it.
1class SongReportService
2 def gather_songs_from_artist(artist_id)
3 songs = Song.where(status: :published)
4 .where(artist_id: artist_id)
5 .order(:title)
6
7 ...
8 end
9end
10
11class SongController < ApplicationController
12 def index
13 @songs = Song.where(status: :published)
14 .order(:release_date)
15
16 ...
17 end
18end
19
20class SongRefreshJob < ApplicationJob
21 def perform
22 songs = Song.where(status: :published)
23
24 ...
25 end
26endIn the example above, we have three use-cases where the Song model is being
queried. In the SongReporterService that is used for reporting data about
songs, we try to get published songs from a concrete artist. Then, in the
SongController, we get published songs and order them by the release date.
And finally, in the SongRefreshJob we get only published songs and do
something with them.
This is all fine, but what if we suddenly decide to change the status name to
released or make some other changes to the way we fetch songs? We would have
to go and edit all occurrences separately. Also, the code above is not DRY. It
repeats itself across the application. Don't let this get you down. Luckily, there
are solutions to this problem.
We can use Rails scopes to DRY this code out. Scoping allows you to define
commonly-used queries, which can be called on associations and objects. This makes
our code readable and easier to change. But, maybe the most important thing is that
scopes allow us to chain other Active Record methods such as joins, where,
etc. Let's see how our code looks with scopes.
1class Song < ApplicationRecord
2 ...
3
4 scope :published, -> { where(published: true) }
5 scope :by_artist, ->(artist_id) { where(artist_id: artist_id) }
6 scope :sorted_by_title, { order(:title) }
7 scope :sorted_by_release_date, { order(:release_date) }
8
9 ...
10end
11
12class SongReportService
13 def gather_songs_from_artist(artist_id)
14 songs = Song.published.by_artist(artist_id).sorted_by_title
15
16 ...
17 end
18end
19
20class SongController < ApplicationController
21 def index
22 @songs = Song.published.sorted_by_release_date
23
24 ...
25 end
26end
27
28class SongRefreshJob < ApplicationJob
29 def perform
30 songs = Song.published
31
32 ...
33 end
34endThere you go. We managed to cut the repeating code and put it in the model. But this doesn't always work out for the best, especially if you are diagnosed with the case of a fat model or a God Object. Adding more and more methods and responsibilities to the model might not be such a great idea.
My advice here is to keep scope usage to a minimum and only extract the
common queries there. In our case, maybe the where(published: true) would be
a perfect scope since it is used everywhere. For other SQL related code, you could use
something called a Repository pattern. Let's find out what it is.
Repository Pattern
What we are about to show is not a 1:1 Repository pattern as defined in the Domain-Driven Design book. The idea behind ours and the Rails Repository pattern is to separate database logic from business logic. We could also go full-on and create a repository class that does the raw SQL calls for us instead of Active Record, but I wouldn't recommend such things unless you really need it.
What we can do is create a SongRepository and put the database logic in there.
1class SongRepository
2 class << self
3 def find(id)
4 Song.find(id)
5 rescue ActiveRecord::RecordNotFound => e
6 raise RecordNotFoundError, e
7 end
8
9 def destroy(id)
10 find(id).destroy
11 end
12
13 def recently_published_by_artist(artist_id)
14 Song.where(published: true)
15 .where(artist_id: artist_id)
16 .order(:release_date)
17 end
18 end
19end
20
21class SongReportService
22 def gather_songs_from_artist(artist_id)
23 songs = SongRepository.recently_published_by_artist(artist_id)
24
25 ...
26 end
27end
28
29class SongController < ApplicationController
30 def destroy
31 ...
32
33 SongRepository.destroy(params[:id])
34
35 ...
36 end
37endWhat we did here is we isolated the querying logic into a testable class.
Also, the model is no longer concerned with scopes and logic. The controller
and models are thin, and everyone's happy. Right? Well, there is still Active
Record doing all the heavy pulling there. In our scenario, we use find, which
generates the following:
1SELECT "songs".* FROM "songs" WHERE "songs"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]The "right" way would be to have all this defined inside the
SongRepository. As I said, I would not recommend that. You don't need it and you
want to have full control. A use case for going away from Active Record would
be that you need some complex tricks inside SQL that are not easily supported
by Active Record.
Talking about raw SQL and Active Record, I also have to bring up one topic. The topic of migrations and how to do them properly. Let's dive in.
Migrations — Who Cares?
I often hear an argument when writing migrations that the code there should not be as good as it is in the rest of the application. And that argument doesn't sit well with me. People tend to use this excuse to set up smelly code in the migrations since it will only be run once and forgotten. Maybe this is true if you are working with a couple of people and everyone is in constant sync all the time.
The reality is often different. The application can be used by a larger number of people not knowing what happens with different application parts. And if you put some questionable one-off code there, you might break someone's development environment for a couple of hours because of the corrupted database state or just a weird migration. Not sure if this is an anti-pattern, but you should be aware of it.
How to make migrations more convenient for other people? Let's go through a list that will make migrations easier for everyone on the project.
Make Sure You Always Provide a Down Method
You never know when something is going to be rolled back. If your migration is
not reversible, make sure to raise ActiveRecord::IrreversibleMigration
exception like so:
1def down
2 raise ActiveRecord::IrreversibleMigration
3endTry to Avoid Active Record in Migrations
The idea here is to minimize external dependencies except for the state of the database at the time when the migration should be executed. So there will be no Active Record validations to ruin (or maybe save) your day. You are left with plain SQL. For example, let's write a migration that will publish all songs from a certain artist.
1class UpdateArtistsSongsToPublished < ActiveRecord::Migration[6.0]
2 def up
3 execute <<-SQL
4 UPDATE songs
5 SET published = true
6 WHERE artist_id = 46
7 SQL
8 end
9
10 def down
11 execute <<-SQL
12 UPDATE songs
13 SET published = false
14 WHERE artist_id = 46
15 SQL
16 end
17endIf you have a great need for the Song model, a suggestion would be to define it
inside the migration. That way, you can bulletproof your migration from any
potential changes in the actual Active Record model inside the app/models.
But, is this all fine and dandy? Let's go to our next point.
Separate Schema Migrations From Data Migrations
Going through the Rails Guides on migrations, you'll read the following:
Migrations are a feature of Active Record that allows you to evolve your database schema over time. Rather than write schema modifications in pure SQL, migrations allow you to use a Ruby DSL to describe changes to your tables.
In the summary of the guide, there is no mention of editing the actual data of the database table, only the structure. So, the fact that we used the regular migration to update songs in the second point is not completely right.
If you need to regularly do something similar in your project, consider using
the data_migrate gem. It is a nice way
of separating data migrations from schema migrations. We can easily rewrite our previous example
with it. To generate the data migration, we can do the following:
1bin/rails generate data_migration update_artists_songs_to_publishedAnd then add the migration logic there:
1class UpdateArtistsSongsToPublished < ActiveRecord::Migration[6.0]
2 def up
3 execute <<-SQL
4 UPDATE songs
5 SET published = true
6 WHERE artist_id = 46
7 SQL
8 end
9
10 def down
11 execute <<-SQL
12 UPDATE songs
13 SET published = false
14 WHERE artist_id = 46
15 SQL
16 end
17endThis way, you are keeping all your schema migrations inside the db/migrate
directory and all the migrations that deal with the data inside the db/data
directory.
Final Thoughts
Dealing with models and keeping them readably in Rails is a constant struggle. Hopefully, in this blog post, you got to see the possible pitfalls and solutions to common problems. The list of model anti-patterns and patterns is far from complete in this post, but these are the most notable ones I found recently.
If you are interested in more Rails patterns and anti-patterns, stay tuned for the next installment in the series. In upcoming posts, we'll go through common problems and solutions to the view and controller side of the Rails MVC.
Until next time, cheers!
P.S. If you'd like to read Ruby Magic posts as soon as they get off the press, subscribe to our Ruby Magic newsletter and never miss a single post!
