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

Michael Groß edited comment on IMAGING-159 at 1/31/15 8:42 PM:
---------------------------------------------------------------

@[~britter] Don't worry, maybe [~ebourg] is right. The builder-pattern approach 
requires the user to know/remember of which type a parameter value is to get 
the value. I completed a second proposal at

http://github.com/mgmechanics/commons-imaging/tree/IMAGING-159-POJO

which uses the POJO/inheritance pattern as he pointed out. I did not apply the 
new classes (org.apache.commons.imaging.ImagingParameters/~Jpeg/~Tiff) but I am 
willing to do this if my patch is accepted.

For some values I found default values, for some not. For PARAM_KEY_VERBOSE I 
assumed Boolean.FALSE. If there are more default values, please let me know. 
Each default value means one isXyzPresent() method less.

I prevented getters from return null. You may ask via isXyzPresent() if a value 
was set so you never get the RuntimeException. "if (xyz == null)" would mean to 
know that null means "this parameter didn't got a value yet". "if 
isXyzPresent()" seems to be more obvious.


was (Author: mgmechanics):
@[~britter] Don't worry, maybe [~ebourg] is right. The builder-pattern approach 
requires the user to know/remember of which type a parameter value is to get 
the value. I completed a second proposal at

http://github.com/mgmechanics/commons-imaging/tree/IMAGING-159-POJO

which uses the POJO/inheritance pattern as he pointed out. I did not apply the 
new classes (org.apache.commons.imaging.ImagingParameters/~Jpeg/~Tiff) but I am 
willing to do this when my patch is accepted.

For some values I found default values, for some not. For PARAM_KEY_VERBOSE I 
assumed Boolean.FALSE. If there are more default values, please let me know. 
Each default value means one isXyzPresent() method less.

I prevented getters from return null. You may ask via isXyzPresent() if a value 
was set so you never get the RuntimeException. "if (xyz == null)" would mean to 
know that null means "this parameter didn't got a value yet". "if 
isXyzPresent()" seems to be more obvious.

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

Reply via email to