kinow commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r673491489



##########
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:
       Hi @gwlucastrig 
   
   An approach similar to what we are trying here, thanks for sharing! 
   
   >This would compile just fine. However, betaParameters would contain 
information that was irrelevant to the alpha parser. It would probably be a 
case where the programmer made a mistake. The program would probably run, but 
it might not do quite what the developer hoped. Some debugging would be 
necessary. How could we help this case? 
   
   I think we have reached the same point here. While both our approaches lead 
to no generics, the cost is accepting compilation and having to do some runtime 
checking, though not perfect.
   
   >Maybe forget the whole bit about the copy constructor for AlphaParameters. 
I think most classes would just do the branching approach and only the guys who 
needed to do something special would use the copy-constructor approach.
   
   My current experiment is not going anywhere, so I'll `git reset --hard` and 
start testing yours with the copy constructor.




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


Reply via email to