collectiveidea / interactor

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

Thoughts about using #call instead of #perform #27

Closed zmoazeni closed 10 years ago

zmoazeni commented 10 years ago

Hey folks,

I was wondering if you had discussed using MyInteractor.call(foo: bar) instead of perform.

The reason I ask is that I use these object mostly as MethodObjects, and within Ruby an object that responds to call fit that mold very well.

A hidden benefit of that would being able to mix in lambdas and methods within an organized list.

class PlaceOrder
  include Interactor::Organizer

  def self.something_else_minor
    # another minor behavior that has an arity of 0
  end

  organize [
    CheckInventory,
    ->(context){ 
      # something minor that does not warrant a full interactor,
      # and has access to the context
    },
    ChargeCard,
    CreateOrder,
    DeliverThankYou,
    DeliverOrderNotification,
    method(:something_else_minor),
    ScheduleFollowUp
  ]
end

class CreateOrder
  include Interactor

  def call
    # can keep this design the same as #perform with an arity of 0

    # same logic that would go in perform
  end
end
toreyheinz commented 10 years ago

:+1:

toreyheinz commented 10 years ago

It could be as simple as creating an alias. I do like perform, but there are times where call would make more sense. Having the option would be nice.

jasonroelofs commented 10 years ago

I don't like this idea for two reasons

1) It's a fundamental change to the API of the library and would have to be an added feature, adding complexity to the tool (#call vs #perform, pass in context vs implicit access).

2) It goes against the very reason for these Interactor objects in the first place: well named units of code that contain single business cases. Forcing the use of full objects strongly encourages thinking about naming. If you can just drop a bunch of MethodObjects or plain procs into an Organizer list, you lose all of that naming benefit. Also, an array of procs IMO looks ugly and is very hard to read.

Also, as a 2.5, this change discourages building reusable code. Interactors are meant to be on their own and can be combined in Organizers but don't have to. Adding a proc or method object to an Organizer list is code that cannot be reused anywhere else.

toreyheinz commented 10 years ago

I don't think breaks the well named units of code that contain single business cases aspect of the Interactor, but it would change the way an organizer would work. Maybe we need a different type of Organizer?

zmoazeni commented 10 years ago

If you can just drop a bunch of MethodObjects or plain procs into an Organizer list, you lose all of that naming benefit. Also, an array of procs IMO looks ugly and is very hard to read.

Well, you could certainly take it to that extreme, but I'm not encouraging that and I don't think this change encourages that. If anything, it follows ruby semantics a little closer.

Personally, I'm looking at this for edge case 1 liners that don't make sense to live inside the previous interactors so I wouldn't have to resort to this:

class PlaceOrder
  include Interactor::Organizer

  class TinyInteractor
    include Interactor
    def perform
      # a one liner that could be a proc
    end
  end

  organize [
    CheckInventory,
    TinyInteractor,
    ChargeCard,
    CreateOrder,
    DeliverThankYou,
    DeliverOrderNotification,
    ScheduleFollowUp
  ]
end
jasonroelofs commented 10 years ago

I'm not sure what you mean by "ruby semantics". What's wrong with a single-line Interactor? We have a couple of those in our projects and they work great. Our Organizers are a list of names lining out exactly what they do. Adding proc support would break that nice naming structure.

One important aspect of keeping code clean is keeping abstractions on the same level. Organizers are at the level of "Here's what I do", and each Interactor is at the level of "this is how it's implemented". Mixing those makes code harder to read, understand, and maintain.

bryckbost commented 10 years ago

What's wrong with a single-line Interactor? We have a couple of those in our projects and they work great.

One line, buried in a method, in a class that might not ever get reused by anything else can at times feel overkill.

There's some merit to what @zmoazeni is saying, though it does feel like a bit of a departure from the Organizer naming structure.

mikekelly commented 10 years ago

the protocol for this kind of thing in ruby is a call method, using perform was a mistake in the original design of this library.

We could move to call and alias perform to it. We could also add a deprecation warning when client code calls perform.

robinroestenburg commented 10 years ago

Another benefit (not mentioned in this thread yet) of using call instead of perform is that you are able to substitute an interactor or organizer by a simple lambda in your specs.

sagmor commented 10 years ago

I took an alternative approach to this and put it on #39.

zmoazeni commented 10 years ago

Closing this since it's being publicly discussed along with other changes in #44