Bug 49696 - Default ImageWriterRegistry instance register method does not override image writers
Summary: Default ImageWriterRegistry instance register method does not override image ...
Status: RESOLVED FIXED
Alias: None
Product: XMLGraphicsCommons - Now in Jira
Classification: Unclassified
Component: image writer (show other bugs)
Version: Trunk
Hardware: All All
: P2 minor (vote)
Target Milestone: --
Assignee: XML Graphics Project Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-03 19:17 UTC by Joshua Marquart
Modified: 2010-09-08 05:25 UTC (History)
1 user (show)



Attachments
Adds method to ImageWriterRegistry to allow priority when registering writers (2.41 KB, application/octet-stream)
2010-08-03 19:17 UTC, Joshua Marquart
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Marquart 2010-08-03 19:17:00 UTC
Created attachment 25835 [details]
Adds method to ImageWriterRegistry to allow priority when registering writers

For the singleton instance of ImageWriterRegistry, there is no way to register a new writer to override an already-registered MIME type.

ImageWriterRegistry retrieves writers based on a high-to-low priority, and checks whether the writer is functional.

The text of the ImageWriterRegistry register method reads "If an ImageWriter for the same target MIME type has already been registered, it is overwritten with the new one.", but this is not a true statement.  The writer is instead placed in an internal MIME type-specific array based on a high-to-low preference priority.

Priorities are assigned based on the class name.  Other than adjusting the class loader or altering the package-level default-preferred-order.properties file, there is no way to add a new ImageWriter class name with a priority to the registry.

New writers added via the existing register method will always receive a priority of 0, and built-in classes for existing MIME types (JPEG, TIFF, PNG) will always be preferred.

Proposal is 
1) leave the existing register method which preferences a higher priority over a lower priority alone, but adjust the JavaDoc to state "If an ImageWriter for the same target MIME type has already been registered, it is placed in an array based on priority."

2) add a new method, register(ImageWriter writer, int priority) which will add the priority for the writer class name to the preferred order map prior to calling register(ImageWriter).
Comment 1 Jeremias Maerki 2010-09-02 03:57:46 UTC
Thanks for that patch, Joshua. Applied: http://svn.apache.org/viewvc?rev=991835&view=rev

I've also changed the default order to use negative numbers so the default register() method already overrides the existing implementations:
http://svn.apache.org/viewvc?rev=991838&view=rev
Comment 2 Joshua Marquart 2010-09-02 17:02:45 UTC
Did a code comparison of checked-in vs. patch.

Line 
 import java.util.HashMap;import java.util.Map;
was removed in favor of 
    private Map imageWriterMap = new java.util.HashMap();

Not that it matters to me which way is used, but is there a preference that I don't know about to using new qualified.class.Name() instead of import qualified.class.Name;
Comment 3 Vincent Hennebert 2010-09-03 06:49:23 UTC
(In reply to comment #2)
> Did a code comparison of checked-in vs. patch.
> 
> Line 
>  import java.util.HashMap;import java.util.Map;
> was removed in favor of 
>     private Map imageWriterMap = new java.util.HashMap();
> 
> Not that it matters to me which way is used, but is there a preference that I
> don't know about to using new qualified.class.Name() instead of import
> qualified.class.Name;

Actually I'd be curious to know the rationale too :-)

Vincent
Comment 4 Jeremias Maerki 2010-09-07 09:33:40 UTC
You can read up on the discussion from 2002 right here:
http://markmail.org/thread/n3gp3og37xuzrlio

The FOP Java best practices note that, too:
http://xmlgraphics.apache.org/fop/dev/conventions.html#java-best-practices
Comment 5 Joshua Marquart 2010-09-07 18:08:23 UTC
(In reply to comment #4)

I think you misunderstood the question, Jeremias.

We understand that assigning a class implementation as the interface (assigning a HashMap to Map) is a core Java Best Practice.

What the initial question (which is really not that important) is:

Why was the following code used:

Map mapObject = new java.util.HashMap();

instead of

import java.util.HashMap;

Map mapObject = new HashMap();

Was there a benefit to doing this, and if so, what is it because we're openly curious and would probably apply the practice of not using "import" in our own code if there was a benefit.
Comment 6 Jeremias Maerki 2010-09-08 05:25:44 UTC
(In reply to comment #5)

Ah, I see. Sorry for being slow. I guess it's a matter of personal preference. I like shorter import sections and not having implementation details there may make the whole thing less cluttered. It's not like this is very important to me but I like to think that this reinforces the idea behind prefer-interface-over-impl. I hope that makes sense.