Add ability to halt interactor

[TheSmartnik]

There are times when need to stop executing organizer without making it a failure. Here is a possible solution

This feature was also requested in #118

[TheSmartnik]

@laserlemon Hi, can you please check this one? Would love to hear your feedback.

[laserlemon]

@TheSmartnik Thank you for the contribution! I've definitely considered something similar, but there are a few design details that concern me:

1. Raising an error as a means of flow control doesn't feel right, especially when we're saying this case is explicitly not a failure. See: http://www.virtuouscode.com/2014/05/21/jim-weirich-on-exceptions/, specifically:

   Exceptions should not be used for flow control, use throw/catch for that. This reserves exceptions for true failure conditions.

   You could argue that raising an error was a bad design decision for failure too and I'd have a hard time disagreeing. In general, I'd like to explore moving to throw/catch for flow control in Interactor, probably for failure, but definitely for a new non-failure concept like halting.

2. This may be a moot point after my first point, but the fact that the Halt error class inherits from Failure is confusing if Halt isn't to be treated as a failure. If someone were ever to rescue Interactor::Failure in their application code, a halting scenario would also be rescued and the developer would need to have this special knowledge in order to handle the rescued error properly.

3. The halting concept requires an interactor to have knowledge outside of itself. Consider an interactor deeply nested within an organizer. Interactor is designed in such a way that an interactor can stand alone; it can be called individually or within the context of an organizer without any special considerations.

   With the addition of halting, I can't imagine a scenario where I would write context.halt! within my interactor if I weren't writing that line specifically to affect the behavior of the containing organizer rather than the interactor in question.

   Interactors are designed to have two outcomes, success and failure. Halting adds a third pseudo-state that's related to success, but that only exists to affect its organizer. For that reason, I'd rather explore an implementation that controls flow for this type of scenario from the organizer.

Again, thank you for putting forth the effort on this. I don't think this specific implementation is best in terms of design, but I'm happy to be convinced otherwise or to consult on another effort to introduce similar functionality.

[TheSmartnik]

@laserlemon Thanks for your feedback

1. I totally agree that using exceptions for a flow control is a bad idea. It just looked a little better with current design. That said before submitting pr I tried both options.

2. A possible use context.halt! outside of Organizer is when you need to prevent after_hooks from executing. So I believe, it's ok to leave this concept inside of context

I can redo this pr, without use of exception. If you find that it's alright to leave it inside of Context

[TheSmartnik]

@laserlemon Anyway, if you think that skipping after_hooks isn't good enough argument. I can try to add this functionality though Orginizer, but I'll need your help on this. If you could elaborate your thoughts on possible design, that would be of great help.

Did I understood you correctly and you want this to be used exactly as it now, but halt! will work only when called inside of organizer?

[laserlemon]

Well, I have lots of thoughts on this. It would be no small task. The gist is:

class PlaceOrder
  include Interactor::Organizer

  organize CalculateTotal, CalculateTax
  organize CalculateShipping, allow_failure: true, after_failure: -> { context.shipping = 0 }
  organize CheckInventory
  organize CheckForFreeOrder, allow_failure: true, after_success: -> { context.succeed! }
  organize ChargeCard
end

This adds the concept of an options hash that can be passed as a second argument when calling an interactor. From the organizer's perspective, organization of individual interactors can be broken out to different lines so that options can specified that will be passed when calling those interactors. Some possible options (off the top of my head):

:allow_failure
:before
:after
:after_success
:after_failure
:after_error
:skip_rollback
:if
:unless

This also adds the concept of a hard success. This is similar to your halting concept except that it throws/catches only to skip past the rest of the current interactor's execution. You can see how the organizer might use this to detect success of an individual interactor and choose to skip the rest of its own execution (the remaining interactors).

Again, this represents a big change and would need to be encapsulated in a new major version.

While I'm talking about a new major version, I'm also considering:

1. Dropping support for Rubies < 2 so we can make use of Module#prepend. That way MyInteractor.call(some_context) could behave identically to MyInteractor.new(some_context).call complete with callbacks, etc.
2. Replacing usage of raise for flow control with throw and catch.

3. Rethinking how variables set on the context are made available to the Interactor#call method, perhaps by the developer defining keyword arguments to be explicit about what the method can/must accept:

   class MyInteractor
     def call(foo:, bar: "baz")
       # …
     end
   end

Lots of thoughts! What are your thoughts?

[jbmeerkat]

@laserlemon I can help with a major version. Can you describe in details what do you want to change? Maybe you can make an issue or issues with description where we can discuss changes and I'll write some code.

[laserlemon]

@jbmeerkat Thank you! My plan is to carve out some time in the near future to spin up a GitHub project describing an outline for the next major version and we can write some stories to populate that project.

[jbmeerkat]

@laserlemon what about the major release? Have you thought about it?

and I can implement feature that @TheSmartnik asked for. Is it OK if I follow your ideas during implementation?

[laserlemon]

@jbmeerkat Yes, I've thought about it a lot and have put quite a bit of work into it. You can check out the project. Progress is on the v4 branch.