collectiveidea / interactor

Interactor provides a common interface for performing complex user interactions.
MIT License
3.36k stars 212 forks source link

Remove "magic" context accessors #47

Closed ersatzryan closed 10 years ago

ersatzryan commented 10 years ago

Another confusing bit is that most often, context values are accessed from within an instance of an interactor and the interactor allows direct method access to context values. The original intent here is to make values easily accessible from the controller. The unfortunate side effect is the temptation to use direct access from within the interactor itself for convenience.

class SessionsController < ApplicationController
  def create
    AuthenticateUser.perform(email: params[:email], password: params[:password])
  end
end

class AuthenticateUser
  include Interactor

  def perform
    user = User.find_by(email: email) # <- Magical access to context[:email]

    if user && user.authenticate(password) # <- Magical access to context[:password]
      context[:user] = user
    else
      fail!
    end
  end
end

For a newcomer to Interactor, it's not at all apparent where the email or password methods originate.

Proposed Solution

From within an interactor there will be no magical access to the underlying context. Accessing the context will be explicit.

class AuthenticateUser
  include Interactor

  def call
    user = User.find_by(email: context.email) # <- No magic!

    if user && user.authenticate(context.password) # <- No magic!
      context.user = user
    else
      fail!
    end
  end
end
errinlarsen commented 10 years ago

:+1:

By keeping this as "magic"-free as possible, I can easily re-add the magic I personally like (https://github.com/collectiveidea/interactor/pull/44#discussion_r12199228) back into my Interactors with a simple mixin. Maybe even roll out a "interactor_context_accessors" gem so others can, too.

ersatzryan commented 10 years ago

@errinlarsen funny you should say that @laserlemon and I just had that same discussion. If we keep Interactor as simple as possible it allows others to fill their own needs.

trestrantham commented 10 years ago

:+1: here but this issue feels tightly coupled with #46 (which I also :+1:). Should we combine them?

laserlemon commented 10 years ago

@trestrantham I think they're separate issues. The "magic" direct access to the context is independent of what that context actually is.

trestrantham commented 10 years ago

:+1:

ecielam commented 10 years ago

I do like the simple approach that lets us define additional behaviors. Something similar to what @errinlarsen suggested in this comment or similar to strategies in Decent Exposure provides a framework with default behavior that is easy and clear to extend or replace

laserlemon commented 10 years ago

This issue is about these existing lines of code specifically, not about the actual behavior of the underlying context.

For discussion on the underlying context object, see #46.

laserlemon commented 10 years ago

The proposed solution is included in the v3 branch.