Message-passing, layers and SRP

example.rb

class InviteController < ApplicationController
  # The controller's responsibility is to build an object graph
  # that will do the hard work.
  def accept
    response  = InvitationResponse.new(self)
    scenario  = InvitationScenario.new(response)

    scenario.accept_invite_token_for_user(current_user, params[:token])

    # The alternative is to implement both
    #
    #     inviting_succeded([User])
    #     inviting_failed([Exception])
    #
    # and to replace the 4 lines above with this:
    #
    #     scenario  = InvitationScenario.new(self)
    #     user_repo = InviteRepository.new(scenario)
    #
    #     scenario.accept_invite_token_for_user(params[:token], current_user)
  end
end

# This class supplements "AcceptInvite". It will tell the data repository
# to find some data, and based on the outcome of the retrieval, will do some
# work and then tell its collaborator to act.
# This is where the business logic would go
class InvitationScenario
  def initialize(collaborator, repository = InviteRepository.new(self))
    @collaborator = collaborator
    @repository   = repository
  end

  def accept_invite_token_for_user(current_user, token_str)
    @repository.find_user_by_token(current_user, token_str)
  end

  def user_found(user_record)
    # Maybe send emails, write some audit logs, etc.
    collaborator.inviting_succeeded(user_record)
  end

  def user_not_found(exception)
    # Will tell the police
    collaborator.inviting_failed(exception)
  end

  def find_user_failed(exception)
    # Will tell your mom, something was really wrong
    collaborator.inviting_failed(exception)
  end
end

# This class tells its collaborator about the outcome of
# the data retrieval (it encapsulates ActiveRecord, which shouldn't
# leak in the controller)
class InviteRepository < Struct(:collaborator)
  def find_user_by_token(current_user, token_str)
    begin
      user = current_user.invites.find_by_token!(token_str)
      collaborator.user_found(user)
    rescue ActiveRecord::RecordNotFound => exception
      collaborator.user_not_found(exception)
    rescue => e
      collaborator.find_user_failed(exception)
    end
  end
end

# This class is a proxy to the Rails collaborator. It is used to avoid
# cluttering the Rails collaborator's code with too many methods.
class InvitationResponse < Struct(:controller)
  def inviting_succeeded(user)
    controller.redirect_to invite.item, notice: "Welcome!"
  end

  def inviting_failed(error)
    controller.redirect_to '/', alert: "Oopsy!"
  end
end

# The models looks like
class Invite < ActiveRecord::Base; end
class User < ActiveRecord::Base
  has_many :invites
end

Oh, I see, so instead of changing the method to user_found_by_token you follow the convention of user_found and use the instance variables right? I was a bit confused why you did that, and not still convinced if this is better over being explicit.

However what raised my eyebrows is the hardcoded TokenConsuptionScenario and InviteRepository in user_found method. How would you test that and why did you do that in first place?

Also I would probably go with token_consumed(token) - since you probably can get user from token and remove the user_record from TokenConsumptionScenario or maybe pass it token_consumed(token, user)

The issue I have with this style ((and I was aiming at from the beginning) is you when you have a "imperative" sequence of actions like (find invite by token, mark the token as used, do some other stuff) it's get very verbose and complicated to construct such object graph with collaborators. Not to mention the readability.

I really do like the Tell don't Ask and the fact that you are pushing it to the extreme in a way that basically you don't need a return value. This AFAIK actually is the Actor Model for concurrency btw.

I love this code/ping pong and thanks for any responses so far.

Re: object graph: this is why we need some sort of built-in DI in Ruby. Building the graph manually can't scale. Rubyists tend to think DI is a buzzword, or a nonsensical idea. And it is probably true they don't need it, as the architecture they use (Active Record), wouldn't benefit from any kind of DI. But once you're decoupling things, you can't work without proper DI.

This is why, while trying to prove a point elsewhere (and not TDDing my gists/doing that on top of my head on a Saturday morning :-D), I hardcoded some classes: this is the easy road everybody takes, but IMHO not the one that a medium-to-large apps requires.