Re: [Tm4j-developers] [PATCH] Some improvements for LocatorFactory

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

Hi Florian,

This patch looks good - it certainly cleans up implementation of new 
Locator types for the in-memory backend. I am not sure how easy it will be 
to port this to the Ozone backend. The LocatorBase abstract class should 
probably be modified too (to remove setFactory() and initialise(), although 
it might be necessary to retain something like this for the 
OzoneLocatorBase class (because it is easier to have no-arg constructors 
with Ozone objects) but the necessary casts that would be required would be 
localised within the OzoneLocatorFactoryImpl class, so that shouldn't be a 
problem.

Could you apply the patches and check them in ?

Cheers,

Kal

At 18:19 17/02/2002 +0100, Florian G. Haas wrote:
>Hello.
>
>I've hacked up some changes to LocatorFactory-related classes and would ask
>anyone who's interested to evaluate them. Just take a look -- I'd greatly
>appreciate any comments. Note that this is a patch against the current
>status of the CVS tree, not a TM4J distribution, which is why I only post it
>to this list, not the patch management system at SourceForge. What this
>patch includes:
>
>* A LocatorFactoryBase class, serving as an abstract base class for locator
>factories (who would've guessed), with an implementation of the
>createLocator() method, a generic getImplementation() method returning the
>implementing class for a given notation, and overloaded
>removeImplementation() and registerImplementation() convenience methods with
>String instead of Class arguments.
>* An in-memory LocatorFactoryImpl altered to extend LocatorFactoryBase.
>* An additional test in LocatorFactoryTest with a dummy Locator
>implementation to make sure everything works.
>
>I think the LocatorFactoryBase class with the implemented createLocator()
>method and the abstract getImplementation() method makes locator factory
>implementations a lot more straightforward. Extending LocatorFactoryBase
>means that the implementing class only has to define ways to register and
>remove implementations, and to return a corresponding implementing class for
>a locator notation. No need to worry about actually creating locators.
>
>LocatorFactoryBase.createLocator() uses a bit of metaclass hacking in order
>to actually instantiate the locator. If you're interested, please take a
>look at it and tell me how you feel about this. What it assumes is that any
>valid locator implementation has a constructor with a (LocatorFactory,
>String, String) signature. I think this is reasonable because locators
>should not be created outside the context of a factory and should always
>have a notation and address. If it can't detect such a constructor,
>createLocator() throws a LocatorFactoryException with an explanatory detail
>message. With this approach, it obsoletes the use of initialise() and
>setFactory() on the locator altogether, and I think that's a Good Thing
>because it ultimately means that we could do away with these methods, and a
>locator without a factory, notation, and address would never exist. The way
>things are now, we allow locators to float in free space without an owning
>factory and in an unitialized state, and this IMHO is downright ugly. :-)
>There are obviously some disadvantages to my alternative, too -- such as the
>fact that an implementation design error (not providing a decent
>constructor) will only be detected at run-time, not compile-time. This calls
>for making the constructor requirement absolutely clear in the Locator
>interface documentation. So we have to weigh alternatives here. Certainly no
>hard feelings on my part if we leave things the way they are now.
>
>Furthermore, I think a getImplementation() method should actually be
>specified by the Locator interface. But that's a detail.
>
>- PATCH INSTRUCTIONS (just for completeness' sake)
>
>* Applying the patch
>1. Unzip/untar the attached archive in the root directory of your "tm4j" CVS
>module.
>2. Run "./tm4j-net-patch-2002-02-17" This will patch the ant buildfile and
>call the "tm4j-net-patch-2002-02-17" target. On Windows, you'll have to
>patch the build file manually and then run that target.
>3. Build the "tm4j-net-build" and/or "tm4j-net-test" target.
>
>* Un-applying the patch
>Either restore the affected files
>(src/org/tm4j/net/memory/LocatorFactoryImpl.java and
>src/org/tm4j/net/test/LocatorFactoryTest.java) with "cvs update -C", or
>overwrite the patched files with the ".orig" files in the same respective
>directories. Then build tm4j-net-build.
>
>Hope you like it! :-)
>
>-- Florian