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

Proposal: Add functors that iterate values only #263

Open
oriengy opened this issue Nov 14, 2022 · 9 comments
Open

Proposal: Add functors that iterate values only #263

oriengy opened this issue Nov 14, 2022 · 9 comments

Comments

@oriengy
Copy link

oriengy commented Nov 14, 2022

Functors like "Map" is handy, but can we make it better?

Say we have
func Map[T any, R any](collection []T, iteratee func(T, int) R) []R

I guess we would write code like this often.
keys := lo.Map(channels, func(channel typed.Channel, _ int) string { return makeKey(channel) })

It could be even better if we add an alias to Map functor, that iterate values only.
func MapV[T, R any](collection []T, iteratee func(T)R) []R

So we can turn the previous code into this.
keys := lo.MapV(channels, makeKey)
It would make the code much clear.

And it can be implemented easily like this.

func MapV[T any, R any](collection []T, iteratee func(T) R) []R {
	result := make([]R, len(collection))

	for i, item := range collection {
		result[i] = iteratee(item)
	}

	return result
}

And other functors like Reduce, Filter should benefit from these alias too.
Would this contribution be appreciated @samber ?

@wirekang
Copy link
Contributor

wirekang commented Nov 17, 2022

How about adding a function that ignores second argument? It's more generic, and don't have to make aliases of many functions.

keys := lo.Map(channels, lo.Ignore1(makeKey))

func Ignore1[T0 any, T1 any, R any](f func(T0) R) func(T0,T1) R {
        return func(v0 T0,_ T1) R {
                return f(v0)
        }
}

@oriengy
Copy link
Author

oriengy commented Nov 17, 2022

I think it's a good compromise.

I thought about the NoIndex solution after I issued this.

func NoIndex[T, R any](f func(T) R) func(T, int) R {
	return func(t T, _ int) R {
		return f(t)
	}
}

The "Ignore" functor is better for sure.
And this kinds of functors can do, but I don't think it's clear enough.

Let's consider
keys := lo.Map(channels, makeKey)
it's the most common functional programming style, quite intuitive.

While lo.Map(channels, lo.Ignore1(makeKey))
it's confusing to see a "Ignore" functor here.

I think the alias way is complicated for library developers, and the "Ignore" way is complicated for library users.
I believe that library developers should eat the complexity for users when the price is affordable.

@wirekang

@wirekang
Copy link
Contributor

wirekang commented Nov 17, 2022

I agree with you. lo.Ignore is way too complicated for what it does. (Also users should specify all generic types. lo.Map(channels, lo.Ignore1[typed.Channel, int, string](makeKey)) )

@oriengy
Copy link
Author

oriengy commented Nov 21, 2022

@samber What's your opinion on this proposal? Glad to hear that.

@MaksimSkorobogatov
Copy link

MaksimSkorobogatov commented Mar 14, 2023

It's really too tedious to constantly wrap any function passed to lo.Map to add the index param, when in a lot cases we would simply use existent converting function as second argument to lo.Map.

I think adding lo.MapV (and the same V-suffix to Reduce, Filter, etc) is a good idea) Btw, I can proposal one another variant: the suffix WI (lo.MapWI), meaning "without indexing". I gess it will help to not mix up with other words and suffixes, because "WI" is a rarely used pair of letters.

@peterlgh7
Copy link

peterlgh7 commented Mar 17, 2023

We often have a MapToDTO(p Person) PersonDTO mapping function, and when we need to map a slice of objects we'd have to do lo.Map(persons, func(p Person, _ int) PersonDTO { return MapToDTO(p)}). If the index wasn't in the signature, it would be the much leaner lo.Map(persons, MapToDTO). We use this library extensively in our codebase already, but this is the only reason why we still keep a custom Map implementation.
Not sure what the best solution is though. Aliases are OK but they do bloat the API. Maybe instead of aliases like MapV we could have a subpackage lov where we have index-less implementations of Map, Filter, etc?

@erpaher
Copy link

erpaher commented Apr 26, 2023

#259 as example

@askreet
Copy link

askreet commented Jul 31, 2023

Having just written the same NoIndex transform and thrown it away for lack of clarity and only needing it once, I think something like MapV would be a really good solution here.

@sebwalle
Copy link

sebwalle commented Sep 5, 2023

Yes please, the extra argument makes the usage often no more readable or easy to use than the loop variant of the same functionality.
I think the V suffix is nicer than a separate package as in #259, would be a bit confusing to see noindex.Map() in the code IMO.

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

No branches or pull requests

7 participants