aaronmallen/activeinteractor

[RFC] A more DSL friendly approach

aaronmallen opened this issue · 5 comments

For version 2.0 I'd like to make activeinteractor a little more DSL friendly:

On the current version of activeinteractor (1.0.4) you can do something like this:

class Tweeter < ActiveInteractor::Base
  def perform
    puts "Tweeting..."
    result = Twitter::Client.send(context.message)
    context.fail!(result.body) if result.status != :ok
  end
end

class Facebooker < ActiveInteractor::Base
  def perform
    puts "Posting to Facebook..."
    result = Facebook::Client.send(context.message)
    context.fail!(result.body) if result.status != :ok
  end
end

class SocialMediaInteractor < ActiveInteractor::Organizer::Base
  organize :tweeter, :facebooker
end

result = Tweeter.perform(message: "My Tweet")
"Tweeting..."
result.success?
#=> true

result = SocialMediaInteractor.perform(message: "My Post")
"Tweeting..."
"Posting to Facebook..."
result.success?
#=> true

I'd like developers to instead be able to do something more akin to this:

class SocialMediaInteractor < ActiveInteractor::Base
  perform :tweet do
    puts "Tweeting..."
    result = Twitter::Client.send(context.message)
    state.fail!(result.body) if result.status != :ok
  end

  perform :facebook do
    puts "Posting to Facebook..."
    result = Facebook::Client.send(context.message)
    state.fail!(result.body) if result.status != :ok
  end
end

result = SocialMediaInteractor.tweet(message: "My Tweet")
"Tweeting..."
result.state.success?
#=> true

result = SocialMediaInteractor.perform(message: "My Post")
"Tweeting..."
"Posting to Facebook..."
result.state.success?
#=> true

Additionally it should be noted status reporting has been abstracted into a new object called state to reduce the responsibility of the context object.

The underlying benefit is developers would now be able to either invoke all perform methods on a given interactor (i.e. SocialMediaInteractor.perform(...)) or specific methods (i.e. SocialMediaInteractor.tweet(...)) and still have the value of attribute validation and callbacks.

To address the issue of maintaining rollback functionality the implementation could look something like this:

class SocialMediaInteractor < ActiveInteractor::Base
  perform :tweet do
    puts "Tweeting..."
    result = Twitter::Client.send(context.message)
    state.fail!(result.body) if result.status != :ok
  end

  rollback :tweet do
    ...
  end

  perform :facebook do
    puts "Posting to Facebook..."
    result = Facebook::Client.send(context.message)
    state.fail!(result.body) if result.status != :ok
  end

  rollback :facebook do
    ...
  end
end

What benefits does this DSL provide over the current PORO design? Personally, the reason I like something like Pundit vs cancancan is that it's simple and extensible; you don't have to work around the DSL. It's also easy to substitute something else during tests, whereas if each of your interactors follows a different interface (tweet, facebook, etc.) then you can't just stub something out that plays a generic "interactor" role, you have to care about the specific interactor you plug in.

I also think the rollback example is a good one, because in the non-DSL version, you could simply define your own rollback or undo methods. This library doesn't have to care that it's something you want to do. If there are other use cases (repeat, log, etc.), the library doesn't need to add in specific support for it, nor do you need to hack it in yourself: you just define methods. Again if you want to stub something out in your tests, the interactors all play the same role and have the same interface.

If there are use cases you're thinking of that the DSL makes simpler or easier to understand, then that might change things. But the way I see it, this change only adds a slight layer of abstraction for little benefit, and in fact some drawbacks.

In your DSL the SocialMediaInteractor is encouraging mixing environments with differing implantation and promotes mixing the different implementations for FB and Twitter, et.al. into a monolithic class. IMO this encourages developers to include every possible social media task into the same class structure which I fear will just result in yet another 800 line class to scroll through (or to include) to find or use a single interaction point.

Where as the current DSL promotes sparse separation of function and purpose. Each network has its own separation of functions and may still be included in a more monolithic class or dsl wrapper for your application needs, but the developer is guided to separate concerns from the start.

I like the DSL concept perhaps as a seperate grouping construct, but not as the default implementation.

This would make a good gem. But a poor core addition IMO.

I tend to agree with the comments here and am closing this as resolved.