GITNE commented on issue #49: Add support for EXIF 2.32
URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-506266458
 
 
   > On backward compatibility, thanks a lot for taking that into 
consideration. That's an important point.
   > […]
   > I'm writing to that thread about this PR, to confirm whether we are 
keeping BC or not. If not, it would be great if you would be willing to 
complete the work for fully compliance with the standard.
   
   After thinking about it a bit longer, I have realized that this PR already 
does break backwards compatibility in a specific situation. Namely, if an 
application relies on a deprecated `TagInfo` object to be in the 
`ExifTagConstants.ALL_EXIF_TAGS` list. For example, 
`ExifTagConstants.ALL_EXIF_TAGS.contains(ExifTagConstants.EXIF_TAG_EXIF_IMAGE_WIDTH)`
 will return `false` instead of `true` for them. So, if you would simply drop 
in a `commons-imaging.jar` with this patch applied on an existing application 
which expects the `ExifTagConstants.EXIF_TAG_EXIF_IMAGE_WIDTH` to be in 
`ExifTagConstants.ALL_EXIF_TAGS` then things will break. However, I believe 
this to be an exceptionally rare case, if ever implemented in this way. If 
anything `ExifTagConstants.ALL_EXIF_TAGS` has been probably used in iterators 
by applications, so these would still transparently iterate over the new and 
fixed `TagInfo` objects anyway. But they will misbehave if they check on a 
deprecated `TagInfo` object while iterating, yet they will not crash.
   
   We can work around `ExifTagConstants.ALL_EXIF_TAGS.contains()` to return 
`true` on deprecated `TagInfo` objects, however we cannot fix the iterators 
because we do not know which `TagInfo` objects the caller expects. Hence, there 
is practically no way around breaking backwards compatibility. Nevertheless, it 
would be a good thing anyway since IMHO it is better for dependencies/libraries 
to be correct than backward compatible. Consequently, in that situation fixing 
the data types of those primitive static members can be done too.
   
   Furthermore, `ExifTagConstants.ALL_EXIF_TAGS` should have probably been a 
`Set` in the first place because there is really no order among EXIF tags. If 
anything then the current `List` should be reordered by tag ids because there 
is no apparent order in the source code. So, either the lack of order should be 
reflected in the type or the ordered type should have elements in proper order. 
But this might be rather something for another PR or commit because both 
approaches will break backwards compatibility.

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


With regards,
Apache Git Services

Reply via email to