commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McGrady <m...@michaelmcgrady.com>
Subject Re: [chain] CatalogFactoryBase
Date Tue, 19 Oct 2004 05:04:23 GMT
Way cool!  YES!

Michael McGrady

Craig McClanahan wrote:

>On Mon, 18 Oct 2004 10:48:50 -0400, Sean Schofield
><sean_schofield@schof.com> wrote:
>  
>
>>Craig McClanahan wrote:
>>
>>    
>>
>>>I prefer to use them throughout, so we have the opportunity to not synchronize
in cases where thread safety is not an issue -- it is here, though.
>>>
>>>
>>>      
>>>
>>OK makes sense.
>>
>>    
>>
>>>More subtly, though, you'll also note that I use Map instead of
>>>HashMap to declare the actual instance variable, and the return values
>>>      
>>>
>>>from appropriate methods.  That way, the actual implementation class
>>    
>>
>>>could be specialized later without breaking method signatures.  That's
>>>not as easy to do if you're throwing Hashtables around (any
>>>specialized version would have to subclass Hashtable, and not some
>>>more generic interface).
>>>
>>>
>>>
>>>      
>>>
>>This makes sense in general but I don't see how it applies in this
>>specific case.  Only the instance variable is a Map.  There aren't any
>>public methods in CatalogFactory that return Map so how does this help
>>with subclassing?  I'm just wondering ... I'm not proposing changing it
>>back.
>>    
>>
>
>That is true for now, but I've seen many projects (including a couple
>of my own) get bit by adding public accessors later on --
>PropertyUtils.getMappedPropertyDescriptors() returns FastHashMap
>instead of Map and we didn't catch it quick enough :-(.
>
>  
>
>>Finally, I also just noticed that you implemented getInstance() in
>>CatalogFactory and had it return an instance of CatalogBase.  Did you
>>change your mind on this?  Is this because it will be easier to manage
>>class loader issues in the factory?
>>
>>    
>>
>
>Not just easier ... the original strategy I suggested would never
>eliminate the reference to the class loader in the clear() method, so
>the garbage collection problems I spoke of would still happen.  Now,
>clear() -- which is also on CatalogFactory -- throws away its own
>reference to the class loader and any loaded catalogs.
>
>  
>
>>I'm assuming that your reasoning for having CatalogFactoryBase is so
>>that we (or end user) can change the implementation of CatalogFactory
>>without affecting users code.  Right now user's can't really specify
>>their own CatalogFactory.  I'm fine with this, but just checking to see
>>if this was your reasoning.
>>
>>    
>>
>
>Yep, that's the reasoning -- future proofing the fundamental APIs a
>little.  We've had to go back and try to do this thing in other
>scenarios where static utility classes were defined but couldn't be
>replaced.  Now, all we'd need to do if the need came up is add a way
>to register your own CatalogFactory implementation class in order to
>create instances, if the need ever comes up.
>
>The end result of all this, by the way, is quite sweet ... no need to
>pass around references to a Catalog or a CatalogFactory, and it works
>even if [chain] is installed in a shared directory.  You'll be able to
>see this too once you update your CVS checkout to pick up the
>ConfigCatalogRule.java file that I just checked in :-).
>
>  
>
>>sean
>>    
>>
>
>Craig
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>
>
>  
>



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message