Re: [Tm4j-developers] [DONE] Make TopicMapProviderFactory a "real" factory

[Kal A. <ka...@te...>]

On Wed, 2003-11-12 at 21:10, Florian G. Haas wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hello.
> 
> On Wednesday 12 November 2003 12:08, Harald Kuhn wrote:
> | > Well unless Kal objects, I guess the best option is to just add the
> | > org.tm4j.jndi package to the tm4j.sources patternset, so it will be
> | > picked up by the tm4j-build target when compiling.
> |
> | +1
> 
> I just took care of this. Kal, I didn't hear you objecting, but if you do,
> please roll back my latest commit of build.xml.
> 

It works for me :)

> | E.g. in TMNav you could instanciate a new Provider by giving its factoreis
> | classname (inside a swing textfield during runtime). Also i am still not
> | quite sure how to use different providers together (could you please
> | clarify this ?).
> 
> If you want to be sure to get an instance of a particular concrete factory
> class, you instantiate it by invoking its constructor. So, rather than:
> 
> import org.tm4j.topicmap.TopicMapProviderFactory;
> // ... lots of code here
> TopicMapProviderFactory factory = TopicMapProviderFactory.newInstance();
> 
> - -- which would leave it to the abstract TopicMapProviderFactory to select the
> concrete subclass, based on the selection algorithm discussed on this list
> - --, you would use:
> 
> import org.tm4j.topicmap.hibernate.TopicMapProviderFactoryImpl;
> // ... lots of code here
> TopicMapProviderFactoryImpl factory = new TopicMapProviderFactoryImpl;
> 
> or, alternatively:
> 
> String className = swingTextBox.getText(); // pure pseudocode here!
> TopicMapProviderFactory factory =
> 	(TopicMapProviderFactory) Class.forName(className).newInstance();
> 
> That's all. Instantiating the subclass directly is not discouraged per se; you
> only give up backend independence when doing so. If you don't need backend
> independence, no-one forces you to make use of it. :-)
> 

That is more or less how I have documented this in the updated
Developer's Guide.

> | To get around this, I see (if you have other suggestions /
> | ideas, please let me know) two different options: we just use the
> | deprecated methods (not very clean in my opinion) or we add another
> | newInstance method to TopicMapProviderFactory where you can specify the
> | classname in a direct way (e.g. newInstance(String className) or
> | newInstace(Properties props)). The second one may be not in line with the
> | "normal facory behavier" (you seem to know more about that than I) but i
> | think it is much cleaner then using deprecated methods.
> 
> For this very purpose, providing an overloaded newInstance() that takes a
> class name is not necessary, you can simply instantiate the subclass directly
> if you need to; see above.
> 

Right, I agree too - as long as it is clear that concrete subclasses of
TopicMapProvider have to have a no-arguments constructor, then using the
Java Class.newInstance() method is the way to go - I think that it is
common enough practice for it to be familiar to most Java programmers.

> You got me thinking about factory properties, however. I'm beginning to think
> that being able to set configuration properties on the factory object is not
> a bad idea at all. So we could add property-modifying methods to the factory
> base class. That way all concrete subclasses would inherit a means of setting
> and accessing configuration properties for the factory, which would then
> presumably affect all TopicMapProviders subsequently created by that factory.
> How's that sound?
> 

Do you mean default properties that get passed to the
newTopicMapProvider() method ? If so then I agree - just like selecting
which backend to use, specifying the properties for the provider is a
common operation for most applications, so having a standard way of
reading that configuration information from the environment makes sense
to me.

Cheers,

Kal