Create a gist now

Instantly share code, notes, and snippets.

Variation of RubyTapas episode "021 Domain Model Events" without using callbacks
class TasksController < ApplicationController
def update
tracker = TaskTracker.new(@task)
if @task.update_attributes(params[:task])
TaskPusher.new(tracker, socket_id).push_changes
TaskMailSender.new(tracker, current_user).deliver_email
# success response
else
# failure respond
end
end
end
# This TaskTracker class isn't necessary since Rails dirty tracking has a previous_changes
# method that I overlooked. Thanks Jonas Nicklas for pointing this out and blowmage for
# making a fork using this: https://gist.github.com/4060224
#
# Update: One issue with previous_changes is if the TaskPusher saves the model the
# TaskMailSender would not have the original changes. I think this is good enough
# reason to stick with TaskTracker.
class TaskTracker
attr_reader :task, :original_project_id, :original_status, :original_assignee
def initialize(task)
@task = task
@original_project_id = task.project_id
@original_status = task.status
@original_assignee = task.assignee
end
def project_changed?
original_project_id != task.project_id
end
def status_changed?
original_status != task.status
end
def assignee_changed?
original_assignee != task.assignee
end
end
class TaskPusher < Struct.new(:task_tracker, :socket_id)
def push_changes
if task_tracker.assignee_changed?
# push assignee changes
end
if task_tracker.project_changed?
# push project changes
end
end
end
class TaskMailSender < Struct.new(:task_tracker, :recipient)
def deliver_email
if task_tracker.status_changed?
# email status change
end
if task_tracker.assignee_changed?
# email assignee change
end
end
end
@elight
elight commented Nov 10, 2012

The "TaskTracker" is cute (in a good way). My version, https://github.com/elight/rubytapas_21_alternative, just tracks the state within each of the classes that need it. That is, I did the minimum amount of factoring I felt that I could get away with. Assuming that these states would be needed somewhere else, your TaskTracker is a great idea. I may steal that in the future.

@elight
elight commented Nov 10, 2012

Otherwise, yep, we were essentially of one mind. The "TaskPusher" and "TaskMailSender" isolate all of the side effects without any need for callbacks.

@ryanb
Owner
ryanb commented Nov 10, 2012

The primary thing I don't like about the TaskTracker is it duplicates the built in dirty tracking. However since the dirty tracking clears out after the save it's more difficult to use without callbacks.

@ryanb
Owner
ryanb commented Nov 10, 2012

Also callbacks have good support for transactions. If the push/email sender raises an exception it would roll back with a transaction, but here it will not since it happens after the save is committed. I suppose either approach could be desirable.

@elight
elight commented Nov 11, 2012

+1 about TaskTracker not being able to use Dirty tracking. But it's a side effect of ActiveRecord::Dirty being cleared at #save time. Sometimes I wish those changes could be kept in another accessor like "last_saved_changes" or similar. Stll, Avdi was able to rewrite the impl to use AR::Dirty. I'd imagine that any algorithm should be refactorable such that a #last_saved_changes would be unnecessary.

I'd also considered transactions and had exactly the same reaction. The #save call had already occurred. And without knowing more about the external dependencies and the side effects that they may cause, it's hard to know whether it's safe to change the execution order of the code.

@elight
elight commented Nov 11, 2012

But don't be so hard on your own design. TaskTracker serves to untangle some dependencies. Whether or not this makes use of AR::Dirty, it's still a useful refactoring tool sometimes.

In my case, I Extract Method'd everything before I did anything else. You just did one more Extract Class than I did.

I considered doing something about it. If I was going to be an SRP hardass, it was obvious that the predicates didn't really have a business being in those classes. But as reuse of the predicates wasn't necessary in the context of the example, I left them where they lay. ;-)

@jnicklas

@elight: there is such a thing, it's called previous_changes :)

@elight
elight commented Nov 12, 2012

@jnicklas, @blowmage: Gee, Rails had a feature and I didn't know about it? This never happens... facepalm

@ryanb
Owner
ryanb commented Nov 12, 2012

@jnicklas nice, I didn't realize previous_changes existed!

@ryanb
Owner
ryanb commented Nov 12, 2012

Just realized a big issue with using previous_changes. What if the TaskPusher ends up saving the task again? Then the TaskMailSender would not detect the earlier changes. The TaskTracker class seems like a nice solution now.

@avdi
avdi commented Nov 12, 2012

When an object can tell other objects about events that are happening to it as they happen, it eliminates all this extra machinery for tracking changes. Pulling that logic out:

  • removes that knowledge from the most obvious place for it to be - inside the object which is experiencing the events.
  • makes it much more likely that the logic will be duplicated, as other objects also dig into the object's state (ask) instead of letting it emit events (tell).
  • Commits Task to always publicly support the full AM::Dirty interface.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment