[
https://issues.apache.org/jira/browse/IMAGING-159?focusedWorklogId=672593&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-672593
]
ASF GitHub Bot logged work on IMAGING-159:
------------------------------------------
Author: ASF GitHub Bot
Created on: 01/Nov/21 09:44
Start Date: 01/Nov/21 09:44
Worklog Time Spent: 10m
Work Description: 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
Issue Time Tracking
-------------------
Worklog Id: (was: 672593)
Time Spent: 11h 10m (was: 11h)
> 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: 11h 10m
> 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)
