PragTob / after_do

after_do allows you to add simple callbacks to methods
MIT License
114 stars 7 forks source link

Manipulate "args" in before callback #25

Open svoop opened 5 years ago

svoop commented 5 years ago

As of now, it's not possible to manipulate the args in the before callback due to the arguments being passed with a splat *args.

However, there are use cases where it would be very useful to do so, most notably when using before callbacks on setter methods. Here's a very simple setter in a gem of mine:

class Runway
  def remarks=(value)
    @remarks = value&.to_s
  end
end

Unfortunately, I'm dealing with very flaky data sources here which is why I have to deal with less than 1% of the data "manually". The least intrusive way to do so are setter callbacks which kick in when nilis about to be assigned:

Runway.before :remarks= do |value, method, object|
  if value.nil?
    value = "data read from a fixture"
  end
end

This does not work, but I see two ways to make it work:

  1. The callback receives the args without splatting which is a breaking API change. On the other hand, it should be a step towards supporting keyword arguments (haven't tried this though).
  2. Use throw to signal the return value of the callback should be used as arguments for method (similar to Rails' throw :abort). This is a non-breaking API change and would look something like this:
Runway.before :remarks= do |value, method, object|
  if value.nil?
    throw(:args, "data read from a fixture")
  end
end

Would you consider to merge such a feature?

svoop commented 5 years ago

Rather than drilling open after_do, I've implemented a barebone callback in my gem doing just the bits I need there:

module MyGem
  module Callback
    def before(method, &block)
      aliased_method = :"__#{method}"
      alias_method aliased_method, method
      private aliased_method
      define_method method do |*args|
        updated_args = block.call(self, method, args)
        updated_args ||= args
        send(aliased_method, *updated_args)
      end
    end
  end
end
PragTob commented 5 years ago

Hi there,

thanks for the report. I'll reopen this to keep it in mind for the future. I haven't worked on after_do in a long time but might have some time coming up next year to consider this.

Thanks for providing a usage example. my first instinct was to say "nope" (as it's also not exactly what AOP usually does) but seems like an interesting enough use case that if it was easily supportable I'd happily consider it.

So, thanks :+1: