collectiveidea / interactor

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

Make inputs explicit #92

Closed sporto closed 7 years ago

sporto commented 9 years ago

After using interactor for a while we have found that inputs to our services are unclear unless they are documented thoroughly.

For example:

def call
    if user = authenticated_user
      context.user = user
      context.token = user.secret_token
    else
      context.fail!(message: "authenticate_user.failure")
    end

    private

    def authenticated_user
        User.authenticate(context.email, context.password)
    end
  end

In something like this is not obvious that you need to pass an email and a password to the service. It would be nice if inputs are made explicit, even if this means more code e.g.


  def initialize(user: , password:)
    @user = user
    @password = password
  end

def call
    if user = authenticated_user
      context.user = user
      context.token = user.secret_token
    else
      context.fail!(message: "authenticate_user.failure")
    end

    private

    def authenticated_user
        User.authenticate(@email, @password)
    end
  end
tim-kozak commented 9 years ago

For now i use comments like this. But you point is right

class CreateDevice
  include Interactor
  # @param name
  # @param type
  # @return context
  def call
    context.device = Device.create({
                                             name: context.name,
                                             type: context.type
                                         })
    unless context.device
      context.fail!
    end
  end
end
mingan commented 9 years ago

We wrote a small module

module Interactor::Preconditions
  def context_must_have(*fields)
    before do
      fields.each do |field|
        unless context.respond_to? field
          context.fail!(exception: Interactor::IncompleteContextError.new(field))
        end
      end
    end
  end
end

Then used:

class CreateDevice
    include Interactor
    context_must_have :name, :type

    def call
        # ...
    end
end

You can see that it only tests presence of the field in the context and nothing else, but saves us some boilerplate code and documents the use as well.

gabelimon commented 9 years ago

+1. I usually end up with a whole lot of

def call
  context.fail!(error: 'Param is required') unless context.param.present?
  context.fail!(error: 'Other param is required') unless context.other_param.present?

Validation interactors are a good place for this but it would be nice to have some built-in functionality to get rid of the boilerplate. My checks are almost always on presence (object type if I'm feeling particularly strict that day) and my reaction is always failing with an error.

Yoshyn commented 9 years ago

Complety agree. A built-in functionality will be great.

gabelimon commented 9 years ago

I really like the preconditions module suggested above.

sb4m commented 9 years ago

+1

PikachuEXE commented 9 years ago

Sometimes I even want output to be explicit which means input things will not be included in output by default

gabelimon commented 9 years ago

Actually, it looks like something like this is already happening in another issue https://github.com/collectiveidea/interactor/issues/67 https://github.com/collectiveidea/interactor/pull/82

mingan commented 9 years ago

Nice, @gabelimon, totally missed it! I will have a look at Troupe, because form a quick glance it looks very elegant.

danlo commented 8 years ago

I nominate rails_param! annotation for defining & validating parameters. see:

https://github.com/nicolasblanco/rails_param

yjukaku commented 8 years ago

@Mingan The code you posted above looks really promising. Do you need to include Interactor::Preconditions into every Interactor, though?

mingan commented 8 years ago

@yjukaku We haven't forked this gem, so pretty much yes. (However, quite often the inclusion is indirect through a class which defines some shared preconditions for multiple interactors.)

yjukaku commented 8 years ago

😢 I thought maybe you have done some sort of meta programming magic to automatically pull in Interactor::Preconditions whenever Interactor is included.

yjukaku commented 8 years ago

@Mingan I used your code and added a few more checks to it, if you want to take a look at my PR #113

laserlemon commented 7 years ago

Closing in favor of the proposed introduction of keyword arguments to the call instance method in Interactor 4.0. The call method would directly receive a subset of the context based on what's available in the context and what keyword arguments are being requested in the method definition:

class AuthenticateUser
  include Interactor

  def call(email:, password:, remember_me: false)
    # …
  end
end