samber / lo

💥 A Lodash-style Go library based on Go 1.18+ Generics (map, filter, contains, find...)
https://pkg.go.dev/github.com/samber/lo
MIT License
17.31k stars 791 forks source link

Proposal: Add functors that iterate values only #263

Open oriengy opened 1 year ago

oriengy commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

MaksimSkorobogatov commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

https://github.com/samber/lo/pull/259 as example

askreet commented 1 year ago

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 commented 1 year ago

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.

jstrater commented 3 months ago

@samber, if you're interested, I'd be happy to put together a PR for MapV and the rest of the index-free higher-order slice functions.

husam-e commented 3 months ago

Been wanting this for a while as well! Gentle ping @samber, or if there are other maintainers who can opine on moving forward with this proposal and PR :)