Clarify the list of candidate methods for implicit Converters

[struberg]

Good afternoon!

We have 2 possible ways to define the implicit Converter rules (#13):

A.) Either we can pick up only the list of combinations (name + param Type) of the Types in the JDK. This is the wording used in microprofile-config:

• the target type {@code T} has a {@code public static T of(String)} method, or
• the target type {@code T} has a {@code public static T valueOf(String)} method, or
• the target type {@code T} has a public Constructor with a String parameter, or
• the target type {@code T} has a {@code public static T parse(CharSequence)} method

B.) Use any combination of String and CharSequence param Types with a list of method names. This is the wording currently used in ConfigJSR. This list is longer but it is easier to remember for users as it is just a list of method names with String and :

• the target type {@code T} has a {@code static T of(String)} method, or
• the target type {@code T} has a {@code static T of(CharSequence)} method, or
• the target type {@code T} has a {@code static T valueOf(String)} method, or
• the target type {@code T} has a {@code static T valueOf(CharSequence)} method, or
• the target type {@code T} has a {@code static T parse(String)} method, or
• the target type {@code T} has a {@code static T parse(CharSequence)} method, or
• the target type {@code T} has a Constructor with a String parameter, or
• the target type {@code T} has a Constructor with a CharSequence parameter

Now we need more community feedback about which way we should go in ConfigJSR.

[struberg]

Found that joda-time uses parse(String), so we should definitely at least also support this pattern.

[rmannibucau]

technically strong or charsequence difference is trivial to manage so i'd go with both being supported. For the order I would open the door to a very specific config one then standard ones:

1. parseConfigValue(String[CharSequence)
2. parse(String[CharSequence)
3. of(String[CharSequence)
4. valueOf(String[CharSequence)
5. Constructor(String[CharSequence)

[Emily-Jiang]

Can you find any classes to use of, valueOf or Constructor taking String as a parameter? My argument is:

1. If there is no usage, it clashes the spec
2. we can write many tests to test the variety and order override for no apparent reasons.

I would like to just support parse(String), parse(CharSequence), of(String), valueOf(String), Constructor(String).

[rmannibucau]

A lot of libs cover that and more in oss land. Wonder if just having global manually (or spi) registered converters is not enough and then the spec default can be minimalistic as in jaxrs? But i think it should be minimal or complete but not in between taking 2-3 frameworks as a ref (just my opinion of course)

[cruftex]

Go with version B, reason:

In case an implementation wants to support both, String and CharSequence the version A means that two different named method needs to be provided for the semantically identical thing.

Other remarks, maybe already covered elsewhere:

Maybe reduce the list of methods and/or give a clear recommendation for one method name as best practice (probably of....)

I think CharSequence should always be first in the preference order, because its the more lightweight object for the config framework to provide.

The method is not allowed to have a checked exception (of course....). The standard should recommend that an IllegalArgumentException is thrown and the exception message should not include the parsed string.

[Emily-Jiang]

My objection is to we simply list all possible arbitrary methods without knowing whether any class using them. If we put in the spec, we need add tcks to test the order. Think how many tcks we need to test the order etc (if we test all combination, we need to write 8x7x6x5x4x3x2x1 tests to test what we said on the spec).

To be honest, this is convenient way to opt in converters. We don't need to cover 100%. We should aim for majority. In this case, we can stay lean and modern instead of list all possible odd usages.

[rmannibucau]

Emily, tck point doesnt stay long since it is 2 nested loops concretely in the code so 2 lines.

[Emily-Jiang]

I will keep quiet if you show me a class in the shared lib or jdk class using of(CharSequence) or valueOf(CharSequence), constructor(CharSequence). I did not find any usage after some research. My point is not to complicate things if no usage.

[struberg]

We already proved that e.g. Joda time uses one of the methods you said they are not used.

For the valueOf(CharSequende) I just did a quick search and found usages in Spring, the Android SDK, nutrition fw, ct with CharSequence can be found in Apache commons.

And btw, the main argument is not that some libs use them but consistency for the users! It's WAY easier to have a list of methods and both String and CharSequence than having to know a full list of exact matches

[cruftex]

It's WAY easier to have a list of methods and both String and CharSequence than having to know a full list of exact matches

I see that point, but maybe there is some middle ground and the list of methods can be reduced. It's about a standard here. There is no reason why a standard should support distinct libraries. If there is a common pattern used by more than one library then the standard should support that eventually.

Libraries can change and evolve towards a common standard, too. Every library can support the config JSR by either providing a supported generic method or an explicit rule. Joda is outdated and should not be considered for new standards. Spring is moving very fast and adapts quickly.

[rmannibucau]

I join Mark, if the rule is stupid "here are the supported names for the methods and here are the list of supported types and all combinations work" it is way easier to use and understand on user land than having a whitelist to check each time you have a method.

[smoyer64]

Since String implements CharSequence the JDK doesn't need to specify more than CharSequence to allow the method to be called with a String. I guess I'm voting for version (A) since it's more concise and includes any input that implements CharSequence.

[struberg]

@smoyer64 well, the reason is coercion which doesn't automatically happen when using reflection. So you have to call getMethod("of", String.class) AND getMethod("of", CharSequence.class).

It's almost an implementation detail. We only mention both because of portability issues and TCK coverage.

[Emily-Jiang]

agreed with option B. More tcks to be added to test the order of the method listed in the option B to make sure of ->valueOf->parse ->constructor ; string first and then charactersequence.

[smoyer64]

@struberg thanks for the clarification ... I always seem to end up loving and hating reflection! And it certainly doesn't hurt to add implementation hints and gotchas in the specification so long as implementers aren't saddled with the requirement. I guess generically the specification part is that (e.g.) implementations must support parse(CharSequence) for all implementations of CharSequence.