Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] A more DSL friendly approach #229

Closed
aaronmallen opened this issue Jul 29, 2020 · 5 comments
Closed

[RFC] A more DSL friendly approach #229

aaronmallen opened this issue Jul 29, 2020 · 5 comments
Labels
question A question

Comments

@aaronmallen
Copy link
Owner

aaronmallen commented Jul 29, 2020

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.

@aaronmallen aaronmallen added the question A question label Jul 29, 2020
@aaronmallen aaronmallen pinned this issue Jul 29, 2020
@aaronmallen
Copy link
Owner Author

@aaronmallen
Copy link
Owner Author

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

@itsliamegan
Copy link

itsliamegan commented Aug 5, 2020

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.

@codeninja
Copy link

codeninja commented Aug 7, 2020

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.

@aaronmallen
Copy link
Owner Author

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

@aaronmallen aaronmallen unpinned this issue Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question A question
Projects
None yet
Development

No branches or pull requests

3 participants