We are working to add more thorough progress tracking to upcase.com.
Chad and I were pairing on the backend for this feature, and we created a model
Status, that belongs both to a
User and to an
Exercise. This is a
many-to-many relationship that holds a
state attribute. That way we can query,
for each exercise, what its
state is for the currently signed in user.
When a user starts an exercise, we create a
Status object, with a
"Started". If there is no related
Status, then the exercise was not yet
started (and we display to the user
Initially we implemented the following method in the
class Exercise < ActiveRecord::Base def state_for(user) if status = status_for(user) status.state else "Not Started" end end def status_for(user) statuses.where(user: user).order(:created_at).last end end
This made the tests green, and now it was time to refactor. The model was
responsible of knowing what to display in the view for an exercise that was not
"Not Started" string). This was obvious while writing tests. We
moved from integration into unit tests, and discovered we were asserting the
label shown to the user inside of the model spec.
We felt this belonged to the View layer, and moved the method into a helper:
module ExercisesHelper def state_for(user:, exercise:) if status = exercise.status_for(user) status.state else "Not Started" end end end
At this point, Chad and I pushed up our latest changes and opened a pull request. The feedback we received provided us with an even better solution:
The helper’s conditional decides what is the
state for a
Status that doesn’t
yet exist. The
state of a
NullStatus. We chose to name this null object
NotStarted because it makes explicit why we need it, rather than just
denoting what it is.
The third and best implementation we found is:
class NotStarted def state "Not Started" end end class Exercise < ActiveRecord::Base def status_for(user) statuses.where(user: user).order(:created_at).last || NotStarted.new end end
In the view we always show
state, and the corresponding object will always
<%= exercise.status_for(current_user).state %>
We Replaced Conditional with Polymorphism. No wonder the reviewer was also the author of the Polymorphism blog post, thank you Joe! Thank you twice.
Upcase subscribers get access to the Upcase GitHub repo, where they are welcome to see all new features, improvements, and refactorings as we make them.
If you already have access, you may check the pull request with the whole discussion (PR #914). Otherwise, you can always get access to this and more thoughtbot content and repositories through Upcase.