gwlucastrig commented on issue #72: URL: https://github.com/apache/commons-imaging/pull/72#issuecomment-616913125
I thought I should point out one aspect of the new API elements that I think is not optimal. When I added the getFloatingPointRasterData() method to the TiffDirectory class, I had it take ByteOrder as an argument. I took this approach because the existing "getTiffImage" methods did the same thing. But as I look at the unit tests and example applications that use it, it seems awkward to make the code handle a data element that is extraneous to the task that it is trying to accomplish. It's just one extra thing for a developer to have to keep track of and doesn't give him any added capability. I've done some research and studied the TIFF specification, and it's clear that byte order is only specified one place in a TIFF file and doesn't change once the header section of the file is read. So I think a better approach to the new functionality would be to make the TiffDirectory class have byte-order as a member element that was populated by the constructors. Then the getFloatingPointRasterData() would not need it as an argument. Since I've already said that I've "finished" this Pull Request (pending your review), I won't burden you with more changes... But I may submit the change as a future PR, if you think it a good idea. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
