xmolecules / jmolecules-integrations

Technology integration for jMolecules
Apache License 2.0
85 stars 23 forks source link

Priortize constructors over factory methods in converters #191

Closed ooraini closed 1 year ago

ooraini commented 1 year ago

The following line:

return creatorMethod.isPresent() ? creatorMethod : detectConstructor(target, source);

selects the factory method of if it's present over the constructor. Seems harmless until you you have a hierarchy of classes/interfaces in which some of them define the method of. I was surprised when I got ClassCastException because the factory method in the super interface returned an instance of a different type:

interface Super {
  static Super of(String s) {
    // return either new A, or new B
  }
}

record A(String s) implements Super {}
record B(String s) implements Super {}

Is it more sensible to prefer the constructor when present over the factory method? The constructor is guaranteed to return an instance of the same type.

odrotbohm commented 1 year ago

Good catch! I've revised the lookup to only consider the actual target class so that it'd naturally fall back to the concrete class' constructor.