[
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)
