commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Emmanuel Bourg (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (IMAGING-159) There should be a Parameters class
Date Fri, 30 Jan 2015 12:55:34 GMT

    [ https://issues.apache.org/jira/browse/IMAGING-159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14298594#comment-14298594
] 

Emmanuel Bourg commented on IMAGING-159:
----------------------------------------

Ok let me elaborate. You are a random user of commons-imaging and you'd like to tweak the
parameters. Of course just like any good user, you don't read the Javadoc and you discover
the API as you type the code in your favorite IDE.

With the current code, the user understands he has to build a Map<String,Object>, but
he has no idea of the actual keys expected and the values allowed. He has to investigate the
Javadoc until he stumbles on the ImagingConstants interface where the keys and the values
are documented. In this scenario the IDE is unable to suggest a value for the key or the value.

With your proposal, the user understand he has to build a ParameterObject. The constructor
is private and he may not understand immediately a builder pattern is involved. When he figures
it he writes:

    ParameterObject params = ParameterObject.build().set

and expects the IDE to autocomplete. Here the IDE doesn't suggest a property but a setInt,
setFloat, setDouble, setBoolean method... unfortunately at this point he has no idea of the
type expected yet, he doesn't even know the properties available. He picks one type and hopes
it's ok. The first parameter is a Parameter instance and the IDE is able to suggest a value,
so this is better than a plain Map. So he writes:

    ParameterObject params = ParameterObject.build().setInt(Parameter.VERBOSE, 1);

and here the code breaks because he had to write:

    ParameterObject params = ParameterObject.build().setBoolean(Parameter.VERBOSE, true);

Another downside of this design, the enum holding the parameters can't be derived, so the
parameters can't be specialized by image format unless you put them all in the same Parameter
enum and let the user guess the parameters that apply to the format he is interested in.


With a POJO, the user writes directly:

    Parameters params = new Parameters();
    params.setVerbose(true);

It's obvious, it's shorter, the IDE autocompletes happily, there is no guessing of the type
associated with a property, and you can build a hierarchy of parameters (JPEGParameters, PNGParameters,...)
that extend the base Parameters class. It's not immutable but we don't really care (it could
be changed into a fluent style though if it's deemed important, à la CSVFormat)


> There should be a Parameters class
> ----------------------------------
>
>                 Key: IMAGING-159
>                 URL: https://issues.apache.org/jira/browse/IMAGING-159
>             Project: Commons Imaging
>          Issue Type: Improvement
>          Components: imaging.*
>            Reporter: Benedikt Ritter
>             Fix For: Patch Needed
>
>
> Currently options for image I/O are defined as Maps. The leads to the problem that our
code has to validate parameter types when they are used:
> {code:java}
> final Object value = params.get(PARAM_KEY_COMPRESSION);
> if (value != null) {
>   if (!(value instanceof Number)) {
>     throw new ImageWriteException(
>       "Invalid compression parameter, must be numeric: "
>          + value);
>   }
>   compression = ((Number) value).intValue();
> }
> {code}
> This can be simplified if we define a Parameters class that provides additional methods
like {{public int getInt(String key)}}. The implementation could then look up the value from
the map through an exception if it is null or not a number.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message