Simplify/Improve Result object for error situations

[smeriwether]

Problem

I find constructing failure result objects confusing.

class Foo < Chief::Command
  def call
    fail!("My error")
  end
end

result, value, errors = Foo.call

My first read of that code (and others as i've talked to them about it) made me think this would be the result:

result.failure? # true
value           # nil / false
errors          # "My error" or ["My error"]

but instead the result is this:

result.failure? # true
value           # "My error"
errors          #  true

Proposed Solution

Remove the 3rd argument (errors) from the Result object and replace with well named accessor methods.

class BadFoo < Chief::Command
  def call
    fail!("My error")
  end
end

result, value = Foo.call

result.failure? # true
result.error    # "My Error"
value           # "My Error"

class BadFoo2 < Chief::Command
  def call
    fail!("My error", "My other error")
  end
end

result, value = Foo.call

result.failure? # true
result.error    # "My Error, My other error"
result.errors   # ["My Error", "My other error"]
value           # ["My Error", "My other error"]

class GoodFoo < Chief::Command
  def call
    success!("Bar")
  end
end

result, value = Foo.call

result.success? # true
result.error    # nil
value           # "Bar"

Problems

• not backwards compatible
• accepting multiple errors might not be necessary
• accepting a value + error might be a use-case that I can't think of

I'm not at all married to this solution. I would love to hear feedback or other ideas!

[kellywu]

Nice analysis and proposal!

Removing the 3rd argument sounds good to me, I think it's rarely used, or when used, confusing.

Re: Problems

• not backwards compatible
  • seems fine, we can do a major upgrade and dedicate some time to upgrading this
• accepting multiple errors might not be necessary
  • agree that it's unnecessary. i think we can leave it simple and have just one
• accepting a value + error might be a use-case that I can't think of
  • i'm leaning towards the value just being nil in error cases