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 groupByEachUniqueKey to RichIterable #1565

Open
donraab opened this issue Mar 22, 2024 · 6 comments
Open

Add groupByEachUniqueKey to RichIterable #1565

donraab opened this issue Mar 22, 2024 · 6 comments
Assignees

Comments

@donraab
Copy link
Contributor

donraab commented Mar 22, 2024

RichIterable currently has groupBy / groupByEach but only has groupByUniqueKey without the corresponding groupByEachUniqueKey. The method groupByEachUniqueKey would index an item in a Map using multiple keys. Each key will only point to one value. An example of this might be mapping a Person to his/her aliases.

I ran into this problem recently trying to group an Object that contains a distinct range of numbers by each of the distinct numbers. I was able to accomplish this by using a nested injectInto, but it is somewhat awkward and may be difficult to comprehend. In my particular case, it is more optimal using injectInto as I am grouping into an IntObjectMap.

@pranukrish
Copy link

Hi @donraab I'd like to work on this issue. Could you please assign it to me. Thanks!

@donraab
Copy link
Contributor Author

donraab commented Aug 1, 2024

Hi @pranukrish, done!

@pranukrish
Copy link

Hi @donraab! So while trying to implement the functionality I realized that it has to be implemented in all the classes that inherit the RichIterable interface(and that's around a 100 classes) and I wanted to know if there is any work around to implementing this in all of these classes.

@donraab
Copy link
Contributor Author

donraab commented Aug 12, 2024

Hi @pranukrish, yes adding a new method to RichIterable can be very complicated and involve a lot of changes. There is an approach which can split the total amount of work into multiple pieces. If you look at adding groupByEachUniqueKey with a target map similar to groupByUniqueKey, it might be possible to add the implementation as a default method on RichIterable. There will still need to be overrides in a few places, but the return type is not covariant so will always return R extends MutableMapIterable<V, T>.

I think the signature of this version of the method with a target map would look as follows:

    <V, R extends MutableMapIterable<V, T>> R groupByEachUniqueKey(
            Function<? super T, ? extends Iterable<V>> function,
            R target);

Let me know if this makes sense or needs any clarification. Thanks!

@motlin
Copy link
Contributor

motlin commented Aug 12, 2024

The other thing which can make this kind of change easier is searching for similar methods and putting the new method in the same classes. For example, putting this right below each groupByUniqueKey implementation.

@pranukrish
Copy link

pranukrish commented Aug 12, 2024

Thanks for the inputs @donraab and @motlin. I think I'll try a default implementation in RichIterable and see how that's working. And yes, I have implemented it with the same signature you mentioned.
@motlin Yes, I searched for the groupByUniqueKey implementation and that's how I found implementations of this method in around 100 classes. Will try implementing it in RichIterable but if it doesn't work out I will try implementing it in all the classes!

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

3 participants