Scenario
Let’s say that you have an application that has three very similar models, CompactDisc, CassetteTape, and VinylRecord. They all have very similar behaviors, and are all related to one another, though they each have separate tables. STI may or may not be possible.
Code smell Similar to “data clump”, “feature envy”; though in this case it’s models that are similar enough to always be aggregated together.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
# app/models/compact_disc.rb
class CompactDisc < ApplicationRecord
has_many :tracks
end
# app/models/cassette_tape.rb
class CassetteTape < ApplicationRecord
has_many :songs, through: :sides
end
# app/models/vinyl_record.rb
class VinylRecord < ApplicationRecord
has_many :grooves, through: :record_sides
end
They might be collected together like this (where it’s happening isn’t really important):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
@compact_discs = CompactDisc.all
@cassette_tapes = CassetteTape.all
@vinyl_records = VinylRecord.all
(@compact_discs + @cassette_tapes + @vinyl_records).each do |album|
case album.class
when CompactDisc
render partial: "tracklist", collection: album.tracks
when CassetteTape
render partial: "liner_notes", collection: album.songs
when VinylRecord
render partial: "tracklist", collection: album.grooves
end
end
Whether or not this could actually be refactored into an STI relationship is unknown, but we will assume it cannot be, for one reason or another. A case statement, particularly with type-checking, is generally a code smell on its own.
Let’s assume that what we would like the block above to look like is this:
1
2
3
4
5
6
7
@compact_discs = CompactDisc.all
@cassette_tapes = CassetteTape.all
@vinyl_records = VinylRecord.all
(@compact_discs + @cassette_tapes + @vinyl_records).each do |album|
render partial: "album", object: album
end
and that the _album.html.erb partial looks like this (just to show that we’re not just hiding the type checking). This is simplified for clarity.
1
2
3
4
# app/views/albums/_album.html.erb
album.songs.each do |song|
render partial: "song", object: song
end
The most direct change would be to modify the association name, but because this is a legacy app, we want to make the least-impactful change. That We can use the :alias method here. So we might change the models from earlier to look like:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
# app/models/compact_disc.rb
class CompactDisc < ApplicationRecord
has_many :tracks
alias :songs, :tracks
end
# app/models/cassette_tape.rb
class CassetteTape < ApplicationRecord
has_many :songs, through: :sides # unchanged
end
# app/models/vinyl_record.rb
class VinylRecord < ApplicationRecord
has_many :grooves, through: :record_sides
alias :songs, :grooves
end
We can assume that Track and Song quack in similar enough ways that they can function interchangeably in the _song partial. (If not, additional alias can be used)