commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Groß (JIRA) <j...@apache.org>
Subject [jira] [Commented] (IMAGING-159) There should be a Parameters class
Date Tue, 03 Feb 2015 05:11:34 GMT

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

Michael Groß commented on IMAGING-159:
--------------------------------------

[~ebourg] [~britter] I get it why the original author uses a Map<String, Object> to
pass parameters:
{noformat}
// make copy of params; we'll clear keys as we consume them.
params = (params == null) ? new HashMap<String, Object>() : new HashMap<String, Object>(params);

final boolean verbose = Boolean.TRUE.equals(params.get(PARAM_KEY_VERBOSE));

if (params.containsKey(PARAM_KEY_VERBOSE)) {
    params.remove(PARAM_KEY_VERBOSE);
}
if (params.containsKey(BUFFERED_IMAGE_FACTORY)) {
    params.remove(BUFFERED_IMAGE_FACTORY);
}

if (!params.isEmpty()) {
    final Object firstKey = params.keySet().iterator().next();
    throw new ImageReadException("Unknown parameter: " + firstKey);
}
{noformat}
(from org.apache.commons.imaging.formats.bmp.BmpImageParser)

The code reads the value from each present key and then removes the key. At the end of the
process it checks if any keys remain. If so, it throws ImageReadException. This way it checks
if there is a parameter provided which is not applicable in the given class.

*Question*: Would you mind if I omit this check? This could cause that a user may provide
a parameter which has no effect and then ask us why. On other hand, inheritance of the parameter
class could avoid most or even all of these cases because parameters for a JPEG are not mixed
with parameters for PCX i.e. Mind non-applicable parameters in the documentation could do
for corner cases. Omitting the check non-applicable parameters would make the code shorter
and better understandable too. Thus, my match is omitting the check. Can you post here if
you disagree?

> 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: Review Patch
>
>
> 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