kotlin-result icon indicating copy to clipboard operation
kotlin-result copied to clipboard

Consider using consistent "error" naming over "failure"?

Open kirillzh opened this issue 1 year ago • 2 comments

I've noticed that certain APIs use "failure" terminology (onFailure, fold(failure = ), mapBoth(failure =)), whereas others use "error" terminology (mapError, getError, toErrorIf and more).

It looks like it would be more consistent to use "error" terminology across the board: onError, fold(error = ), mapBoth(error = ) + any other APIs that I might be missing.

kirillzh avatar Jul 20 '23 05:07 kirillzh

Not sure if there is a reason behind the wording, but changing this would break the public API for a lot of people using the library. Failure sees to be used for the lambda that is handling the error case.

caiofaustino avatar Jul 20 '23 08:07 caiofaustino

As @caiofaustino mentioned I've generally used "success" and "failure" to be 'actionable' terms, so whenever a lambda is passed it's the lambda that happens on success or on failure, whereas the underlying type is an Ok or an Err. I think this is self-consistent within the library, such that all references of success/failure deal with lambdas in some fashion.

The alternative would be something like mapBoth(transformOk = {}, transformErr = {}) for consistency with the stdlib's map function on collections. This seems like a bit of a mouthful to me.

Success/failure are generally terms that represent the chain of operations as a whole, when you get to dealing with the chaining of multiple result-based operations, I want the reader to be able to distinguish between a single step in the chain being Ok or an Err vs the entire chain finishing successfully or failing.

I'm not against changing everything to be consistent, or simply adding aliases such that onOk is an alias for onSuccess, and onErr is an alias for onFailure, etc. If this is a change you want to make a PR for I'll gladly review it, however seeing all the changes might make us take a step back and decide to keep it the way it is - no promises.

One unintended benefit of this is that they align nicely as they are of the same character length:

doSomething.mapBoth(
    success -> { }
    failure -> { }
)

As an aside, even the stdlib is inconsistent even within the same function, for example the mapTo function has two arguments: destination and transform. One could very easily argue that the args should either be named (to, transform) or (destination, transformation) for consistency.

michaelbull avatar Jul 20 '23 12:07 michaelbull