Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Tue, 3 Feb 2015 05:11:34 +0000 (UTC)
From: =?utf-8?Q?Michael_Gro=C3=9F_=28JIRA=29?= <jira@apache.org>
To: issues@commons.apache.org
Message-ID: <JIRA.12766642.1420897317000.238154.1422940294784@Atlassian.JIRA>
In-Reply-To: <JIRA.12766642.1420897317000@Atlassian.JIRA>
References: <JIRA.12766642.1420897317000@Atlassian.JIRA>
 <JIRA.12766642.1420897317695@arcas>
Subject: [jira] [Commented] (IMAGING-159) There should be a Parameters class
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable


    [ https://issues.apache.org/jira/browse/IMAGING-159?page=3Dcom.atlassia=
n.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D143=
02778#comment-14302778 ]=20

Michael Gro=C3=9F commented on IMAGING-159:
--------------------------------------

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

final boolean verbose =3D 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 =3D 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 Imag=
eReadException. This way it checks if there is a parameter provided which i=
s not applicable in the given class.

*Question*: Would you mind if I omit this check? This could cause that a us=
er may provide a parameter which has no effect and then ask us why. On othe=
r 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 c=
ode shorter and better understandable too. Thus, my match is omitting the c=
heck. 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 pro=
blem that our code has to validate parameter types when they are used:
> {code:java}
> final Object value =3D params.get(PARAM_KEY_COMPRESSION);
> if (value !=3D null) {
>   if (!(value instanceof Number)) {
>     throw new ImageWriteException(
>       "Invalid compression parameter, must be numeric: "
>          + value);
>   }
>   compression =3D ((Number) value).intValue();
> }
> {code}
> This can be simplified if we define a Parameters class that provides addi=
tional methods like {{public int getInt(String key)}}. The implementation c=
ould 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)