kinow commented on pull request #116: URL: https://github.com/apache/commons-imaging/pull/116#issuecomment-956081611
@darkma773r Thanks a lot for creating that pull request. Really clever the intermediate `GenericImageParser`. I liked it and might use it later! :+1: A few questions/notes, feel free to point if any of what I say is off the mark. 1. If the `JpegImageParser` requires a parameter different than the `TiffImageParser`, I assume we would have to create a new `JpegImagingParameter`. In that case, wouldn't we have to keep the old constructor for backward compatibility until a new major release? 2. I think it would be best if cases like this were not allowed `PcxImagingParameters parser = new PcxImagingParameters(new TiffImagingParameters()); // no error in compile or runtime` 3. I think you removed the option to allow `null` parameters in the `Imaging#writeImage` method (and other methods). I thought about doing that too, that would remove one generic suppress I think, but there are unit tests and examples. The API has allowed it since Sanselan, so not too sure about enforcing parameters to be non-null now. 4. I liked the error in this one `new PngImageParser().writeImage(null, null, new TiffImagingParameters());`, which gives `java.lang.IllegalArgumentException: Invalid imaging parameters: expected type...` Your design really solves the generics problems, so I will take another look later and see if I find a middle way. See what you think about the `null` parameters, and allowing the empty parameters types for `Jpeg` and other parsers, or about the other notes @darkma773r :+1: Thanks heaps! ```java Imaging.writeImage((BufferedImage) null, (OutputStream) null, ImageFormats.GIF, new TiffImagingParameters()); // java.lang.IllegalArgumentException: Invalid imaging parameters: expected type org.apache.commons.imaging.common.XmpImagingParameters or a super class but was org.apache.commons.imaging.formats.tiff.TiffImagingParameters :+1: ``` -- 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
