Positional arguments support? about actor HOT 3 CLOSED

Hi @zhengpd! I didn’t think this would be useful, but I guess why not.

Suggestions:

• Instead of introducing an extra has_run_args, perhaps we could use the order in which inputs are added in the first place? (Internally this would be the inputs method.)
• Perhaps we don’t need to raise an error if less args are given, which would allow optional arguments.
• I guess we should also add an extra run method with no ! that would call result instead of call.

I’m open for a PR \o/

from actor.

Hi @sunny , after a second thought, I think it's not much value in supporting positional arguments.

Case A: 4 and more inputs

It's not a good idea to define a method with so many positional arguments, so let's ignore this case.

Case B: 2 or 3 inputs

2 or 3 positional arguments seems reasonable. However, the implementation of positional method (eg. run!) could make the user's project code hard to maintain.

The implementation, going with has_run_args or relying on internal inputs order, would mean that developers have to carefully craft the order of defining inputs. If a developer re-sort the inputs in future, it might cause unnoticeable bug when inputs are the same type. For example:

class DivisionActor < Actor
  input :y
  input :x

  def call; y / x; end
end

Positional call is like Actor.run!(4, 2). Someday someone might think inputs should be defined in alphabetical ASC order and refactor to

class DivisionActor < Actor
  input :x
  input :y

  def call; y / x; end
end

Now the Actor.run!(4, 2) succeeds with incorrect result💥! Such change is easy to be overlooked because the logic is not changed and normally sorting code lines wouldn't cause error. It adds a burden to developers having to know inputs can't be rearranged or new input can't be inserted between old inputs' lines.

So, positional arguments like this just adds more complexity to coding 😞 .

Case C: 1 input

The difference between HiActor.run('Earth') and HiActor.run(planet: 'Earth') is pretty small. Not much value in supporting it by adding more complexity into Actor gem, which I prefer to avoid.

Conclusion

It's better to support positional arguments with some other new solution. I'll close this for now.

from actor.

I appreciate the awesome thought process behind this, and happy that you are considering keeping the codebase simple. I agree with your points! Thank you 🎉

from actor.