[Feature][Solution included] Allows boolean outputs to have `result.boolean?` accessors instead of `result.boolean` about actor HOT 10 CLOSED

Thank you for this issue. I think that adding *? methods would be a great addition! 😍

Minor changes that could work well here:

• I think we could always add them, without the boolean: true option, a bit like ActiveRecord does on all columns.
• The value could be coerced into a boolean when using *? (for example with !!value).
• It could even follow Rails’s conventions of using value.present? to get meaningful coercions of blank strings and arrays into booleans. However I’d rather not tie the gem to ActiveSupport, so this should be an opt-in (for example testing if the #present? method exists or falling back to !!).

Would you want to have a go at a pull-request to have this as a default in the gem? 🙏🏻

Thank you for your reply!. I would gladly try to add them. I shall wok on them ASAP.

from actor.

Yeah, going for the same definition as ActiveSupport sounds good to me!

from actor.

Just opened a PR addressing this at #59!

from actor.

You say that your example code inherits from Actor, but actually it doesn't. It is implemented one level higher, with include ServiceActor::Base. Did your solution not work if you inherit from Actor instead, or was there some other reason you did it this way?

from actor.

Oh!. Sorry my bad. I forgot to replace that. Thank you for pointing that out

I'm not inheritinc from actor but including the ServiceActor::Base as per the docs https://github.com/sunny/actor#build-your-own-actor

class ApplicationActor
 include ServiceActor::Base
end```

I just swapped the composition with inheriting directly from Actor

class ApplicationActor < Actor

  def self.call(options = nil, **arguments)
    result = super options, **arguments

    result.each_pair do |key, value|
      if outputs.dig(key, :boolean)
        result.define_singleton_method "#{key}?", -> { value }
      end
    end
  end
end

And everything works, i ran the tests and all tests are green.

from actor.

Another thing i liked is having Organisers, i.e actors using play to be wrapped within a database transaction by default. Because i found myself playing alot of other actors it made sense to wrap them within an organiser.

To do so, i overrided the Actor#call method to explictly be wrapped within a transaction.

class ApplicationActor < Actor
  def call
    within_transaction { super }
  end

  def within_transaction(&block)
    ActiveRecord::Base.transaction &block
  end
end

This way. All organisers playing other actors will be wrapped within a transaction by default.

For actors that work independently and not as part of an organiser. You can simply call within_transaction and do your method there. Like

class AnyActor < ApplicationActor
  def call
     within_transaction do
       # inside transaction
     end
   end
end

from actor.

Thank you for this issue. I think that adding *? methods would be a great addition! 😍

Minor changes that could work well here:

• I think we could always add them, without the boolean: true option, a bit like ActiveRecord does on all columns.
• The value could be coerced into a boolean when using *? (for example with !!value).
• It could even follow Rails’s conventions of using value.present? to get meaningful coercions of blank strings and arrays into booleans. However I’d rather not tie the gem to ActiveSupport, so this should be an opt-in (for example testing if the #present? method exists or falling back to !!).

Would you want to have a go at a pull-request to have this as a default in the gem? 🙏🏻

from actor.

As for within_transaction I like the way you pulled off having a default transaction for actors using play, smart!

However I think we should rather let users decide where they want the transactions to be. Having transactions inside transactions can lead to misleading code, or make it harder to get out of locks.

Perhaps this could be an option on the play method (play A, B, transaction: true), but then that would only be something to add in actor-rails since it is tied to ActiveRecord. But that wouldn’t shorten your use-case, so maybe it’s best to keep it inside custom actor parents, as you did.

from actor.

As for within_transaction I like the way you pulled off having a default transaction for actors using play, smart!

However I think we should rather let users decide where they want the transactions to be. Having transactions inside transactions can lead to misleading code, or make it harder to get out of locks.

Perhaps this could be an option on the play method (play A, B, transaction: true), but then that would only be something to add in actor-rails since it is tied to ActiveRecord. But that wouldn’t shorten your use-case, so maybe it’s best to keep it inside custom actor parents, as you did.

That approach did not work and i was forced to use something like this. First, i changed ApplicationActor to

class ApplicationActor
  include ServiceActor::Base

  def self.call(options = nil, **arguments)
    result = super options, **arguments

    result.each_pair do |key, value|
      if outputs.dig(key, :boolean)
        result.define_singleton_method "#{key}?" do
          value
        end
      end
    end
  end

  def within_transaction(&block)
    ActiveRecord::Base.transaction &block
  end
end

This way each actor can opt-in for transaction by doing within_transaction { code }

I moved the code related to organisers into a standalone class, that looks like this.

class ApplicationOrganizer < ApplicationActor
  class << self
    def call(options = nil, **arguments)
      ActiveRecord::Base.transaction do
        super options, **arguments
      end
    end

    alias organize play
  end
end

This way. Every organiser(i.e an Actor that plays other actors) will be wrapped within a transaction by default.

from actor.

• It could even follow Rails’s conventions of using value.present? to get meaningful coercions of blank strings and arrays into booleans. However I’d rather not tie the gem to ActiveSupport, so this should be an opt-in (for example testing if the #present? method exists or falling back to !!).

The problem with doing !! it that it will not work exactly like #present?.

!! "" # true
!! "hello" # true

What do you think we should add here to make it work like Rail's #present?. @sunny.

How about we do it like ActiveSupport does it?

https://github.com/rails/rails/blob/ef05f2eda3412c4f4fdabd016699541e5934b062/activesupport/lib/active_support/core_ext/object/blank.rb#L18-L20

I've kinda hit a road-block here!.

      if symbol.end_with?("?") && respond_to?(attribute)
        define_singleton_method symbol do
          !!attribute
        end

        return send(symbol)
      end

from actor.