Variation of RubyTapas episode "021 Domain Model Events" without using callbacks

tasks_controller_refactoring.rb

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

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.

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

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.

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.

+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.

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. ;-)

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

Using previous_changes.

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

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

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.

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.