Support Map fields (-Dkey1=val1 -Dkey2=val2)

[remkop]

One idea is the change the type attribute so that it accepts multiple values. Then, application authors can annotate a Map field like this:

@Option(names = "-D", type = {Integer.class, TimeUnit.class})
Map<Integer, TimeUnit> map;

and the application would be able to parse arguments like this:

<command> -D123=SECONDS -D98=DAYS

[remkop]

Requires converter attribute (instead of type attribute?). #4

[remkop]

I see two ways to implement this:

• Interpreter has special logic for Map fields, and this logic splits the value, and converts each part to the target type based on the type={<class1>, <class2>} annotation attribute.
• Introduce a MapConverter that does the above conversion. This implies an API change: the converter needs to know the types to convert the key and the value to, and it needs access to the type registry to get the converters for these types.

The new API could look like this:

public interface ITypeConverter<K> {
    /**
     * Converts the specified command line argument value to some domain object.
     * @param value the command line argument String value
     * @return the resulting domain object
     * @throws Exception an exception detailing what went wrong during the conversion
     */
    K convert(String value, Field field, Map<Class<?>, ITypeConverter<?>> registry) throws Exception;
}

[remkop]

After prototyping this I don't like the complexity of the new type converter interface. I thought the added power might be a good trade-off for the complexity, but scenarios like #107 show how even more demanding use cases (combining multiple arguments into a single user-defined type) can be catered for without a complex API. (Making the type attribute an array of Classes turns out to be very powerful.)

Keeping the current ITypeConverter interface.
Map fields will be handled by a private method in Interpreter, not by a type converter.

Salvaged from the prototype:

public Map convert(String value, Field f, Map<Class<?>, ITypeConverter<?>> converterMap) throws Exception {
    Class<?>[] classes = getTypeAttribute(f);
    if (classes.length < 2) { throw new IllegalStateException("Field " + f + " needs two types (one for the map key, one for the value) but only has " + classes.length + " types configured."); }
    ITypeConverter<?> keyConverter   = converterMap.get(classes[0]);
    ITypeConverter<?> valueConverter = converterMap.get(classes[1]);
    if (keyConverter == null)   { throw new IllegalStateException("No type converter registered for map key type " + classes[0]); }
    if (valueConverter == null) { throw new IllegalStateException("No type converter registered for map value type " + classes[0]); }
    Map<Object, Object> result = new HashMap<Object, Object>();
    for (String element : split(value, f)) {
        String[] keyValue = element.split("=");
        if (keyValue.length < 2) {
            throw new CommandLine.ParameterException("Value for option " + optionDescription("", f,
                    0) + " should be in KEY=VALUE[,KEY=VALUE]... format but was " + value); // TODO may not be a comma split regex
        }
        Object mapKey = keyConverter.convert(keyValue[0]);
        Object mapValue = valueConverter.convert(keyValue[1]);
        result.put(mapKey, mapValue);
    }
    return result;
}

[pditommaso]

+1

[remkop]

The prototype logic only handles input of the form -Pkey=value or -option key=value. I would like to also support -P key value.

More work is required in the parser, which is why this hasn't been integrated yet.

[pditommaso]

IMO -P key=value would even better than -P key value and likely more straightforward to implement.