kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r678842742
##########
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:
>I'm also leaning toward the "just let it go option" for users that pass
format-specific parameters to parsers of the wrong format type. In my mind, any
format-specific parameters that do not apply should just be ignored.
Yeah, I realize that this would be simpler for us to maintain, but I still
think this could lead to problems to users. There certain formats that people
may be confused about which parameter class to use.
For example, someone could try the TIFF for JPEG, or vice-versa. The
`TiffImagingParameters` extends `XmpImagingParameters`. And the
`JpegImagingParameters` extends the `TiffImagingParameters`. So you could have
code using a Tiff parser with the `JpegImagingParameters` that works fine.
Until we - for instance - override a property like compression in the JPEG
parameter. That could break the code of the user, and would require some
debugging to understand why that started happening.
Maybe we should consult with others in the mailing list?
>Side note: @kinow, I'm looking at the ImagingParameters class and I can't
find any usages of the getImageFormat() method elsewhere in the code. Is that
method used anywhere?
Ah! It's not! Brilliant catch @darkma773r ! The parsers are using the format
to write/read. And a parser knows its own format. No need to have it in the
parameters used in the parser. Thanks a lot for finding this one @darkma773r ,
less one new public API that would need updating later on :+1:
Removing it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
