Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mapping functions for Pair and Triple #5216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

konnik
Copy link

@konnik konnik commented Nov 16, 2023

The Pair and Triple data types lacks a convenient way to map the individual components using a transformation function. The Result type already have map to transform the success value so it is already an established pattern in the stdlib.

For Pair mapFirst and mapSecond are added.
For Triple mapFirst, mapSecond and mapThird are added.

KT-21648 Fixed

@Recordsmen
Copy link

Good Job

*/
public fun <A, B, T> Pair<A, B>.mapFirst(transform: (A) -> T): Pair<T, B> {
contract {
callsInPlace(transform, InvocationKind.AT_MOST_ONCE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transform function is always invoked EXACTLY_ONCE.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transform function is always invoked EXACTLY_ONCE.

Good catch, will fix that.

* function.
* @sample samples.misc.Tuples.pairMapFirst
*/
public fun <A, B, T> Pair<A, B>.mapFirst(transform: (A) -> T): Pair<T, B> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these functions are not inline?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these functions are not inline?

Only because I'm not sure when its suitable to inline functions in Kotlin. :-)
They can of course be inlined if you think that's a good idea? Is there any best practices documented regarding when to inline or not inline functions somewhere?

@fvasco
Copy link
Contributor

fvasco commented Jan 2, 2024

Really Kotlin needs this functions?

Result is a different object, it does not have the copy function, and the map functions contains a bit of logic.
I would happy to see a real use case for this PR.

@konnik
Copy link
Author

konnik commented Jan 4, 2024

Really Kotlin needs this functions?

As a functional programmer I do think so. :-)

This essentially makes the Pair/Triple a functor (or actually a bifunctor/trifunctor) and makes it much more convenient (and less error prone) to work with these types.

Here is an example that I thinks makes it clear:

// say we have a pair of Int to String
val pair: Pair<Int, String> = 42 to "The answer..."

// now we want to transform it into a Pair<Int,Int> instead. This is
// easy if we have the suggested mapping function.
val a: Pair<Int, Int> = pair.mapSecond { it.length }

// to do the same without the mapping functions you have to
// create a new pair from scratch which means needing to copying the first
// element of the pair. 
val b: Pair<Int, Int> = pair.first to pair.second.length
val c: Pair<Int, Int> = Pair(pair.first, pair.second.length) 

// note that we can't use the copy-method since the type has changed
val d: Pair<Int, Int> = pair.copy(pair.first, pair.second.length) // this won't compile

// also note that nothing is preventing us from swapping the values by mistake. This is impossible using the mapping function.
val bug: Pair<Int, Int> = Pair(pair.second.length, pair.first)

IMHO, the winner measured on clarity and conciseness is definitely alternative a.

@konnik konnik reopened this Jan 4, 2024
@konnik
Copy link
Author

konnik commented Jan 4, 2024

Tried to clean up some things in the branch and force pushed some stuff and the PR was closed. Sorry for this, don't know what happened. Reopened the PR.

@fvasco
Copy link
Contributor

fvasco commented Jan 5, 2024

As a functional programmer I do think so. :-)

Does this pull request address a tangible issue?

val bug: Pair<Int, Int> = Pair(pair.second.length, pair.first)

This outcome arises from the misuse of primitive types and the utilization of an anonymous tuple, in my humble opinion.
In this context, first and second lack meaningfulness, except within a very limited scope in the code.
While mapping a value in an Option or an exception in a Result holds significance, I fail to perceive much rationale in mapping the third value of a tuple, especially with a distinct data type. Perhaps it would be more beneficial to create a new tuple to elucidate the content (and the significance) of this data.
Is it advisable to promote this utilization of Pair?

Could you provide a concrete real-life example?

@konnik
Copy link
Author

konnik commented Jan 5, 2024

I agree that excessive use of Pair/Triple should be avoided in favour of more specialised and descriptive types if possible. Nevertheless the Kotlin stdlib does provide them and then should in my opinion try make it as ergonomic as possible to use them (when the user has decided it IS appropriate to use them).

I can't remember the specific use case that led me to initiate this PR (it was a long time ago) but isn't the example I already gave is enough to see the benefits?

I mean, mapping the contents of a "container like structure" without changing the structure itself is something we do all the time in FP. I can't see how the container like structure Pair/Triple is any different than a List, Result, Option or a Parser, they are all functors that can be mapped over.

@fvasco
Copy link
Contributor

fvasco commented Jan 5, 2024

isn't the example I already gave is enough to see the benefits?

No, I don't detect Pair(pair.first, pair.second.length) as a common practice, so I am skeptic about the value of these methods.
I am looking for a reason to suggest to developers to use these methods.

I can't see how the container like structure Pair/Triple is any different than a List, Result, Option or a Parser, they are all functors that can be mapped over.

This is not the point.
I agree with you: it is possible to define a mapping function for all of these data type, this does not imply that it is useful.
If you use often map on Result or List, but you does not remember when you used map on Pair (neither if using map on Pair is a good practice), then these type are different.

This is my point of view.

@konnik
Copy link
Author

konnik commented Jan 5, 2024

I am looking for a reason to suggest to developers to use these methods.

How about this suggestion?

"when you need to transform one element of a Pair (possibly into another type) without changing the other element, then use mapFirst/ mapSecond."

If you use often map on Result or List, but you does not remember when you used map on Pair (neither if using map on Pair is a good practice), then these type are different.

Yes, the types differ, but only in how frequent they are used in a particular codebase. But regarding if a mapping-function is useful or not, they don't differ at all. To be able to use a data structure as a functor is always useful, imho.

I can't really see the problem of introducing these functions to enable a more functional style of programming with Pair/Triple.

@ilya-g
Copy link
Member

ilya-g commented Jan 26, 2024

pair.first to pair.second.length

grep.app finds several usages of this pattern, but not very much:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants