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

ASF GitHub Bot logged work on IMAGING-159:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 29/Jul/21 19:34
            Start Date: 29/Jul/21 19:34
    Worklog Time Spent: 10m 
      Work Description: gwlucastrig commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r679432456



##########
File path: src/main/java/org/apache/commons/imaging/ImageParser.java
##########
@@ -79,25 +78,18 @@
  * that the documentation is perfect, especially in the more obscure
  * and specialized areas of implementation.
  *
- * <h3>The "Map params" argument</h3>
+ * <h3>The "params" argument</h3>
  *
- * Many of the methods specified by this class accept an argument of
- * type Map giving a list of parameters to be used when processing an
- * image. For example, some of the output formats permit the specification
+ * <p>Many of the methods specified by this class accept an argument of
+ * type {@code T} defining the parameters to be used when
+ * processing an image. For example, some of the output formats permit
  * of different kinds of image compression or color models. Some of the
  * reading methods permit the calling application to require strict
- * format compliance.   In many cases, however, an application will not
- * require the use of this argument.  While some of the ImageParser
- * implementations check for (and ignore) null arguments for this parameter,
- * not all of them do (at least not at the time these notes were written).
- * Therefore, a prudent programmer will always supply an valid, though
- * empty instance of a Map implementation when calling such methods.
- * Generally, the java HashMap class is useful for this purpose.
+ * format compliance.</p>
  *
- * <p>Additionally, developers creating or enhancing classes derived
- * from ImageParser are encouraged to include such checks in their code.
+ * @param <T> type of parameters used by this image parser
  */
-public abstract class ImageParser extends BinaryFileParser {
+public abstract class ImageParser<T extends ImagingParameters> extends 
BinaryFileParser {

Review comment:
       Well, I am thinking back to my reaction when Java 1.5 first introduced 
generics.  My first thought was "what a good idea".  My second thought was "of 
all the mistakes I have made coding in Java (and there are a lot of them), I 
don't think I ever tried to store the wrong kind of object in a collection".   
My point here is that we need to simplify things for the user as much as 
possible, but if our own code starts to get too complicated we might be going 
down the wrong road.  Also, I wonder why JpegImagingParameters should extend 
TiffImagingParameters. The TIFF format has MANY Tiff-unique elements (support 
for old fashioned FAX parameters, Photogrammetric Interpreters, odd-duck 
compression standards).  I would think that if there were any overlap between 
the two package branches at all, perhaps JpegImagingParameters and 
TiffImagingParameters might have a common ancestor.   Of course, I have been so 
completely focused on TIFF that I haven't really studied the JPEG package. So I 
am not sure how it is organized.
   
    




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 631299)
    Time Spent: 8h 20m  (was: 8h 10m)

> 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.*
>    Affects Versions: 1.0-alpha2
>            Reporter: Benedikt Ritter
>            Assignee: Bruno P. Kinoshita
>            Priority: Major
>              Labels: github
>             Fix For: 1.0-alpha3
>
>          Time Spent: 8h 20m
>  Remaining Estimate: 0h
>
> 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
(v8.3.4#803005)

Reply via email to