RFC: Redundant code in getPropertyValue

[ArtDu]

https://github.com/tarantool/cartridge-springdata/blob/a7c438e14455626d4671c397b87edd97b495f07b/src/main/java/org/springframework/data/tarantool/core/convert/MappingTarantoolReadConverter.java#L198-L231

We don't need this branch condition: https://github.com/tarantool/cartridge-springdata/blob/a7c438e14455626d4671c397b87edd97b495f07b/src/main/java/org/springframework/data/tarantool/core/convert/MappingTarantoolReadConverter.java#L219-L220

Now I'll try to explain why:

• This is redundant code because it completely overlaps functionality below by this code https://github.com/tarantool/cartridge-springdata/blob/a7c438e14455626d4671c397b87edd97b495f07b/src/main/java/org/springframework/data/tarantool/core/convert/MappingTarantoolReadConverter.java#L223-L225

  • getMap vs getObject in fact, the difference between these methods is that one of them has a targetClass, while the other does not have this knowledge. And the key converter search call for these two functions looks like this:

    For getMap:

    public <V extends Value, O> O fromValue(V v, Class<O> 𝐭𝐚𝐫𝐠𝐞𝐭𝐂𝐥𝐚𝐬𝐬) {
        Function<String, Optional<ValueConverter<V, O>>> getter =
                typeName -> valueConverters.getOrDefault(typeName, Collections.emptyList()).stream()
                        .map(c -> (ValueConverter<V, O>) c)
                        .filter(c -> c.canConvertValue(v))
                        .𝐟𝐢𝐥𝐭𝐞𝐫(𝐜 -> 𝐜𝐡𝐞𝐜𝐤𝐂𝐨𝐧𝐯𝐞𝐫𝐭𝐞𝐫𝐁𝐲𝐓𝐚𝐫𝐠𝐞𝐭𝐓𝐲𝐩𝐞(𝐜, 𝐭𝐚𝐫𝐠𝐞𝐭𝐂𝐥𝐚𝐬𝐬))
                        .findFirst();
        return fromValue(v, getter);
    }

    And for getObject:

    public <V extends Value, O> O fromValue(V v) {
        Function<String, Optional<ValueConverter<V, O>>> getter =
                typeName -> valueConverters.getOrDefault(typeName, Collections.emptyList()).stream()
                        .map(c -> (ValueConverter<V, O>) c)
                        .filter(c -> c.canConvertValue(v))
                        .findFirst();
        return fromValue(v, getter);
    }

    We have checkConverterByTargetType, but what does that mean for converting. We filter only those converters that convert the value to Map.class. Such a converter by default is one DefaultMapValueConverter and it is also the only converter in valueConverters that converts exactly those objects whose class is inherited from MapValue.

    public class DefaultMapValueConverter implements ValueConverter<MapValue, Map<?, ?>> {

    Those only one converter will be found for class objects inherited from MapValue and we also check this only converter if it is really converted to Map.class. Therefore, this filtering is not needed here and we can use getObject without targetClass.

  • convertCustomType overlaps in readValue After calling getMap and after calling getObject we have in HashMap which passed in all cases to convertCustomType. In case without canGetObject(fieldName, Map.class) condition branch we passed this Map in readValue -> convertIfNeeded -> convertCustomType https://github.com/tarantool/cartridge-springdata/blob/a7c438e14455626d4671c397b87edd97b495f07b/src/main/java/org/springframework/data/tarantool/core/convert/MappingTarantoolReadConverter.java#L233-L250 https://github.com/tarantool/cartridge-springdata/blob/a7c438e14455626d4671c397b87edd97b495f07b/src/main/java/org/springframework/data/tarantool/core/convert/MappingTarantoolReadConverter.java#L327-L336

Please advise me if I made a mistake somewhere and it is impossible to remove it.