Skip to content
This repository was archived by the owner on Feb 7, 2018. It is now read-only.

Callbacks or status objects? #67

Open
marten opened this issue Aug 27, 2013 · 14 comments
Open

Callbacks or status objects? #67

marten opened this issue Aug 27, 2013 · 14 comments

Comments

@marten
Copy link
Contributor

marten commented Aug 27, 2013

Right now RoQua adds callbacks to interactors, which are used to communicate result state and execute bits of controller logic (redirect_to @post for success vs render action: :edit for failure):

class PostsController
  def update
    interaction = PostUpdate.new(params)
    interaction.on_success { redirect_to interaction.post }
    interaction.on_failure { render action: :edit, locals: {post: interaction.post} }
    interaction.call
  end
end

class PostUpdate < Interactor
  def on_success(&block)
    @on_success = block
  end
  # same for on_failure

  def execute
    if rand > 0.5
      @on_success.call
    else
      @on_failure.call
    end
  end
end

We could add it like that to Pavlov, but another way would be to support returning "status objects" from interactors:

class PostsController
  def update
    result = PostUpdate.new(params).call
    result.on_success { redirect_to interaction.post }
    result.on_failure { render action: :edit, locals: {post: result.post} }
  end
end

class PostUpdate < Interactor
  class PostUpdateStatus < Struct.new(:status, :post)
    def on_success
      yield if status == :success
    end
    # same for on_failure
  end

  def execute
    if rand > 0.5
      PostUpdateStatus.new(:success, post)
    else
      PostUpdateStatus.new(:failure, post)
    end
  end
end

For us, both would work equally well. Which would be your preference?

@markijbema
Copy link
Member

I think the second one would work better with the helper syntax, and removes the need for a seperate create and call statement. I think we would want to add some syntax to support this though.

@michiel3
Copy link

michiel3 commented Sep 4, 2013

Liking the second option too.

@markijbema
Copy link
Member

actually you can do the first one by just treating the callbacks as parameters, right?

@marten
Copy link
Contributor Author

marten commented Sep 5, 2013

Yeah, and with the 1.9 syntax it's not even that ugly.

@jjoos
Copy link
Member

jjoos commented Sep 5, 2013

Agree on the second one.

@marten
Copy link
Contributor Author

marten commented Sep 5, 2013

Righto, I'll try to whip something up for the second style then.

@markijbema
Copy link
Member

Should we do it explicit like that though, or should we do it rails controller style:

 def execute
    if rand > 0.5
      success post
    else
      failure post
    end
  end

@marten
Copy link
Contributor Author

marten commented Sep 5, 2013

Oh, yeah I want to have some supporting DSL. We currently have "succeed!" and "fail!", but I'd rather make it a bit more generalized and support arbitrary callbacks. Possibly we also want to support returning multiple objects, to support something like https://gist.github.com/marten/9e659cd7629cd2e5d93e#file-interactor-rb-L22 where we have a bunch of things we want to know during rendering, but we couldn't think of a sensible containing model name.

@markijbema
Copy link
Member

I'd like to vote against fail! though, since that's way too similar to fail

@markijbema
Copy link
Member

Agreed about something for multiple return values, we sometimes need to do that as well (and currently use tuples)

@marten
Copy link
Contributor Author

marten commented Sep 5, 2013

I don't want fail!. It doesn't align with succeed!. :P

@marten
Copy link
Contributor Author

marten commented Sep 5, 2013

Okay, did some work on return object and multiple return values, no callbacks yet. Also, obviously not backwards compatible or anything.

class Interactor
  include Pavlov::Operation

  def call
    hash = super
    self.class.return_object.new(hash)
  end

  def self.returns(*args)
    return_object.send(:attribute, *args)
  end

  def self.return_object
    @return_object ||= Class.new do
      include Virtus
    end
  end
end

module Interactors
  class SelectMeasurement < Interactor
    attribute :current_patient,          Patient
    # ...

    returns :protocols
    returns :measurement
    # ...

    def execute
      protocols = query(:available_protocols_with_measurements)
      # ...

      return {protocols: protocols, 
              measurement: measurement,
              questionnaires: questionnaires,
              respondent_type: respondent_type}
    end
  end
end

@markijbema
Copy link
Member

Hmm, I'm not to happy about instantiating new classes all the time. Or openstruct for that matter (which would be a logical replacement). Can we think of a way where we do not need to clear the method lookup cache (runtime)?

@marten
Copy link
Contributor Author

marten commented Sep 5, 2013

Haha, oh that's definitely not what I'm proposing. That was just the easiest way to get the example implemented and passing the interactors' tests, and a way to check how difficult the implementation of such API would be :) I didn't even commit this in RoQua.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants