Compatibility issues while converting Classes to Records
Asked Answered
P

1

12

I have been working with the following class named City

@ToString
@AllArgsConstructor
public class City {
    Integer id;
    String name;
}

and tried to convert it to a record called CityRecord as

record CityRecord(Integer id, String name) {} // much cleaner!

But moving to such a representation, one of our unit tests starts failing. The tests internally deal with a list of cities read from a JSON file and mapped to an object further counting the cities while grouping them under into a Map. Simplified to something like:

List<City> cities = List.of(
        new City(1, "one"),
        new City(2, "two"),
        new City(3, "three"),
        new City(2, "two"));
Map<City, Long> cityListMap = cities.stream()
        .collect(Collectors.groupingBy(Function.identity(),
                Collectors.counting()));

The above code asserted true to contain 4 keys and each accounting for 1 of its occurrence. With the record representation, there are no more than 3 keys in the resulting Map. What is causing this and what should be the way to go around this?

Pyrognostics answered 22/3, 2020 at 7:40 Comment(2)
As a side note, the use of @lombok.EqualsAndHashCode public class City doesn't fix it either! Learnings -- One must have Unit Tests!Pyrognostics
The Lombok code is not modeling an (Integer id, String name) pair as data; it is merely automating away some syntactic boilerplate. It lacks appropriate equals/hashCode methods, so it does not consider two (id, name) equivalent based on their state. The record does. It depends, of course, what you want. In most situations, the Lombok version is probably "broken", but you've not stated your requirements so we can't know.Petterson
P
13

Cause

The reason behind the behavior observed is as documented in java.lang.Record

For all record classes, the following invariant must hold: if a record R's components are c1, c2, ... cn, then if a record instance is copied as follows:

 R copy = new R(r.c1(), r.c2(), ..., r.cn());   then it must be the case that r.equals(copy).

In short, your CityRecord class now has an equals(and hashcode) implementation that compares the two attributes and ensure if they are equal the record consisting of those components are also equal. As a result of this evaluation, the two record objects with the same attributes would be grouped together.

The result, therefore, would be correct to infer/assert that there should be three such keys with the one having id=2, name="two" counted twice.

Immediate Remedy

An immediate temporary solution to this would be to create a custom(flawed - reason explained later) equals implementation within your record representation as well. This would look like:

record CityRecord(Integer id, String name) {

    // WARNING, BROKEN CODE
    // Does not adhere to contract of `Record::equals`
    @Override
    public boolean equals(Object o) {
        return this == o;
    }

    @Override
    public int hashCode() {
        return System.identityHashCode(this);
    }
}

Now that the comparison would be between two objects as in while using the existing City class, your tests would just work fine. But you must note the caution below before using any such remedy.

Caution

As the JEP-359 reads, Records are more like "data carrier" and while choosing to migrate your existing classes, you must be aware of the standard members acquired by a record automatically.

Planning to migrate one must be aware of the complete details of the current implementation, such as in the example you quoted while you've grouped by City, there should be no reason to have two cities with same id and name data to be listed differently. They should be equal, it should be the same data after all repeated twice and hence the correct counts.

In which case, your existing implementation if representing a data model could be rectified to match the record in a way by overwriting the equals implementation to account for comparing the individual attributes as well which is where the immediate remedy stated above is contradictory and should be avoided.

Pyrognostics answered 22/3, 2020 at 7:40 Comment(2)
Always open for a better answer from the experts.Pyrognostics
Not only should you "never have to do this", but in fact, this makes your code broken, because it doesn't adhere to the refined contract of equals() in java.lang.Record. If you want identity equality, then you don't want a record. Records are semantic tools first and syntactic tools only secondarily.Petterson

© 2022 - 2024 — McMap. All rights reserved.