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

user_7804477 0
Viewed 143

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?

collectors java-stream reduction java
2 Answers

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)
user_12215710

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)));
user_78044770
Related