[arrow-core] Make param names more dev friendly

[aballano]

Current state

Currently in many DataTypes functions such as the common fold, have this format:

fun <B> fold(fa: (Throwable) -> B, fb: (A) -> B)

The problem

Is common to find usages taking advantage of named parameters, in order to easily and quickly differentiate one function from the other. Such usages could look as follows with the current implementation:

Try { "Hello" }.fold(fa = {
    // is fa the error or success case?
}, fb = {
    // ditto
})

Proposed solution

My proposal is to be more explicit with param names matching each possible case such as, for the Try example, ifSuccess and ifFailure.

Try { "Hello" }.fold(ifFailure = {
    handleError(it)
}, ifSuccess = {
    print(it)
})

alternatively:

Try { "Hello" }.fold(
    ifFailure = ::handleError, 
    ifSuccess = ::print
)

Scope

I've reduced the scope to only change non deprecated functions and those which have at least two parameters referring to a specific state, if applied.

Current progress:

• Try
• Either

Notes

Feel free to suggest better namings, I've mainly tried to match with Kotlin's similar functions, such as fold in Lists and so on. The rest are on my own subjective opinion.

[JorgeCastilloPrz]

The point is that the rest of the library is coded as fa / fb following usual FP naming conventions. So, even if I could get use to this we would probably need a similar approach for the whole library. That's my concern.

[JorgeCastilloPrz]

What's this about?

[aballano]

Euh, nothing :)

[aballano]

@JorgeCastilloPrz Indeed, we could either release this as per "module-basis" or "whole lib-basis", I personally would prefer the former as I cannot assure being able to work on this for a much longer period :)

[raulraja]

fa and fb are valid in polymorphic contexts like type classes but in concrete data types it's fine to use descriptive names like ifLeft, ifRight. The challenge is that instances and syntax inside type class blocks that affects those data types will still refer to those as fa, fb. For example:

ListK.applicative(). run {
  map(l1, l2, l3, l4, { ... }) // this comes from applicative so it won't have the friendly names
}

[pakoito]

Renaming inherited parameters causes a compiler warning for a good reason, as it conflicts on call-by-name for parameters! It's okay to have different parameter names for datatype functions, but anything in instances has to retain the parent's name.

[aballano]

@raulraja Good to know! Is there anything we could do there from here? Is that ok to go with or a blocker?

[codecov]

Codecov Report

Merging #853 into master will decrease coverage by 0.02%.
The diff coverage is 30.43%.

@@             Coverage Diff              @@
##             master     #853      +/-   ##
============================================
- Coverage     44.88%   44.85%   -0.03%     
  Complexity      628      628              
============================================
  Files           300      300              
  Lines          7709     7709              
  Branches        834      834              
============================================
- Hits           3460     3458       -2     
  Misses         3947     3947              
- Partials        302      304       +2

Impacted Files	Coverage Δ	Complexity Δ
...re/arrow-core/src/main/kotlin/arrow/core/Option.kt	58.06% <0%> (ø)	12 <0> (ø)	⬇️
...core/src/main/kotlin/arrow/core/PartialFunction.kt	92% <100%> (ø)	1 <0> (ø)	⬇️
...s/core/arrow-core/src/main/kotlin/arrow/core/Id.kt	78.57% <100%> (ø)	6 <2> (ø)	⬇️
...re/arrow-core/src/main/kotlin/arrow/core/Either.kt	47.61% <11.11%> (ø)	10 <0> (ø)	⬇️
.../core/arrow-core/src/main/kotlin/arrow/core/Try.kt	70.58% <60%> (ø)	22 <2> (ø)	⬇️
...ics/src/main/kotlin/arrow/optics/instances/mapk.kt	87.5% <0%> (-4.17%)	0% <0%> (ø)
...tics/src/main/kotlin/arrow/optics/instances/map.kt	91.17% <0%> (-2.95%)	0% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cab1fa7...a128199. Read the comment docs.

[aballano]

I'll be merging for now. Hopefully we can continue with the rename in other modules too.