Collectors.reducing method is updating the same identity when used as downstream for Collectors.partitionBy

Question

I have a class similar to the below MyObject.

public class MyObject {
    private String key; // not unique. multiple objects can have the same key.
    private boolean isPermanent;
    private double value1;
    private double value2;
    
    public MyObject merge(MyObject m){
        value1 += m.getValue1();
        value2 += m.getValue2();
        return this;
    }

    // getters, setters and constructors...
}

Following is the sample data:

List<MyObject> objs = new ArrayList<>();

objs.add(new MyObject("1", false, 100, 200));
objs.add(new MyObject("2", false, 300, 100));
objs.add(new MyObject("1", false, 200, 300));

objs.add(new MyObject("3", true, 100, 200));
objs.add(new MyObject("1", true, 500, 100));
objs.add(new MyObject("1", true, 100, 100));

I want to combine these objects based on isPermanent and key and did the following:

(Note that I have added import static java.util.stream.Collectors.* to import groupingBy, partitioningBy and reducing in the below code)

objs.stream()
    .collect(partitioningBy(MyObject::isPermanent,
                            groupingBy(MyObject::getKey,
                                       reducing(new MyObject(), MyObject::merge))));

The returned map is of type Map<Boolean, Map<String, MyObject>>. I am expecting the map returned to be as below (ignoring fields other than value1 and value2)

{false : { 1 : { 300, 500 } } , { 2 : { 300, 100 } }, true : { 1 : { 600, 200 } } , { 3 : { 100, 200 } } }

But the result I am getting is like:

{false : { 1 : { 1300, 1000 } } , { 2 : { 1300, 1000 } }, true : { 1 : { 1300, 1000 } } , { 3 : { 1300, 1000 } } }

Since I am passing an object as the identity, I believe that the same object is being updated for every group. Since a lambda cannot be passed onto the reduction method, is there any way to solve this problem?

Eran · Accepted Answer · 2020-12-28T10:23:54.813

3

You can use a static merge function that returns a new instance:

public static MyObject merge(MyObject one,MyObject two) {
    MyObject merged = new MyObject ();
    merged.setValue1(one.getValue1()+two.getValue1());
    merged.setValue2(one.getValue2()+two.getValue2());
    return merged;
}

You'll have to remove your existing non-static merge method in order for the static method to be picked by the compiler in place of the method reference.

This way, MyObject::merge will produce a new MyObject instance each time.

If you don't want to remove the existing method, you can still add the static method if you replace the method reference with the following lambda expression:

(o1,o2)->MyObject.merge(o1,o2)

Without adding the static method, you can replace the MyObject::merge method reference with the following lambda expression:

(o1,o2)-> new MyObject().merge(o1).merge(o2)

edited Dec 28 '20 at 10:23

answered Dec 28 '20 at 09:25

Eran

387,369
54
702
768

This is a good suggestion, but the issue is that the `merge` function is used in multiple other places which require merging on the same object. Is there any other way to achieve this without any change in the `MyObject` class? – Gautham M Dec 28 '20 at 09:39
1

@GauthamM You can add the static method without removing the non-static method if you replace the method reference with a lambda expression. It would still change the MyObject class, but will not affect code that uses the existing method. – Eran Dec 28 '20 at 09:42
but still that would result in adding a method to the `MyObject`. Can something be done in the `stream-collect-partition` code? – Gautham M Dec 28 '20 at 09:48
1

@GauthamM it can, but it won't be pretty. Let me edit my answer See edit – Eran Dec 28 '20 at 09:49
The last one is fine. – Gautham M Dec 28 '20 at 10:19
1

I have updated it to make it more shorter – Gautham M Dec 28 '20 at 10:24
1

@GauthamM updated your update to make it even shorter :) – Eran Dec 28 '20 at 10:24
2

@GauthamM you could use your original `merge` method by replacing `reducing(new MyObject(), MyObject::merge)` with `Collector.of(MyObject::new, MyObject::merge, MyObject::merge)`. That’s what [Mutable Reduction](https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/stream/package-summary.html#MutableReduction) is about. – Holger Apr 06 '21 at 14:19
@Holger Yes that would be a better approach. A new object would be created only for each grouping. So it avoids the unwanted creation of a new object for each reduction as in this answer. – Gautham M Apr 06 '21 at 14:44

Gautham M · Answer 2 · 2021-04-25T14:55:14.313

Based on Holger's comment in the above answer, instead of using reducing, it would be better to use Collector.of.

For the given List<MyObject> objs in question, the below code using reducing would generate 7 new objects of type MyObject. (A new object is generated as identity and one for each reduction).

groupingBy(MyObject::getKey, reducing((o1,o2)-> new MyObject().merge(o1).merge(o2)));

But using mutable reduction with Collectors, the number of objects created would be reduced to 4, which is the optimal amount of objects that needs to be created for the given input in question. (2 reduced objects each for true and false partitions).

Collector<MyObject, MyObject, MyObject> objCollector = Collector.of(MyObject::new,
                                                                    MyObject::merge,
                                                                    MyObject::merge);

objs.stream()
    .collect(partitioningBy(MyObject::isPermanent,
                            groupingBy(MyObject::getKey, objCollector)));

Collectors.reducing method is updating the same identity when used as downstream for Collectors.partitionBy

2 Answers2

Linked