Martin Desruisseaux wrote:
> Jody Garnett a écrit :
>> FeatureTypeFactory (the geotools interface) cannot be used with 
>> FactoryRegistry and thus made available through CommonFactoryFinder 
>> ... it is too "stateful".
> Note that this is not a blocker. FeatureTypeFactory can be as 
> statefull as you want, providing that the state can be specified by 
> Hints (adding new hints key if needed).
In this case it is a blocker, the factory literally builds a list of 
AttributeTypes and then later creates a FeatureType from them ... it 
really is a builder rather then a factory.
> Note that there is one advantage in specifying the state through hints 
> compared to explicit arguments to constructor: we can add or remove 
> hints in future version without API break. The inconvenient is that 
> the set of hints must be documented in javadoc.
Understood; the above implementation of FeatureTypeFactory is really a 
lost cause. Changing the FM should set it right.
> The ability of adding or removing hints without compatibility break is 
> useful when the hints cascade through many factories. Use case:
>
>    - Factory A depends on factory B.
>    - Factory A constructor passes the user hints to factory B. Example:
>
>      FactoryA(Hints userHints) {
>          myDependency = FactoryFinder.getFactoryB(userHints);
>      }
>
>    - A new hints (Hints.NEW_HINT) is added for factory B.
>
>    - A user try to use Hints.NEW_HINT while instantiating factory A.
>      He expects the Hints.NEW_HINT to be passed to factory B. Example:
>
>      Hints userHints = new Hints(Hints.NEW_HINTS, someValue);
>      myFactory = FactoryFinder.getFactoryA(userHints);
>
>    - With the hints framework, Hints.NEW_HINTS is passed gracefully
>      to factory B without any change in the code.
>
>    - If values were specified explicitly as contructor arguments instead
>      of a map of Hints, as below:
>
>          myDependency = FactoryFinder.getFactoryB(arg0, arg1, arg2);
>
>      then the new user hint would be ignored or unusable until every
>      occurence of FactoryFinder.getFactoryB(...) in the whole Geotools
>      code base is updated to something like:
>
>      FactoryA(Foo0 arg0, Foo1 arg1, Foo2 arg2, Foo3 newArg) {
>          myDependency = FactoryFinder.getFactoryB(arg0, arg1, arg2, 
> newArg);
>      }
>
> In summary, explicit constructor argument means that any change in 
> arguments may cascade in many places because of dependencies, while a 
> single Hints argument cascade gently.
Yep I get it ... this is the style of play that a container promotes as 
well. Basically making the coding experience more forgiving. My problem 
is more that we are doing this work at all (rather then about it working 
or not).

Jody
> This is not a problem if:
>
>  - FeatureTypes has a FeatureTypes(Hints userHints) constructor.
>  - FeatureTypeBuilder(Hints) constructor invoke super(userHints)
>  - FeatureTypeFactory(Hints) constructor invoke super(userHints)
>  - etc.
Nope the problem is the factory has set methods, and literally keeps 
state ... different threads end up getting the same instance and then 
die. The state is ment to be applied later .. it really is just not a 
Factory.
>> All of which has way more state then just "name" ...  and I cannot 
>> figure out how to ask FactoryRegistry not to cache (so I get the same 
>> instance back each time).
> You don't need to ask FactoryRegistry to not cache. Quite the 
> opposite; let him cache.
>
> Your factory should extends, directly or indirectly, AbstractFactory. 
> Suppose that FeatureTypes use a name, an other factory and a foo (I 
> add explicit "this." for clarity):
>
>    FeatureTypes(Hints userHints) {
>        super(userHints);
>        if (userHints != null) {
>            this.name           = userHints.get(Hints.NAME);
>            this.foo            = userHints.get(Hints.FOO);
>            this.anOtherFactory = 
> FactoryFinder.getAnOtherFactory(userHints);
>        }
>        if (this.name == null) {
>            this.name = someDefaultValue;
>        }
>        if (this.foo == null) {
>            this.foo = someDefaultValue;
>        }
>        if (this.anOtherFactory == null) {
>            this.anOtherFactory = someDefaultValue;
>        }
>        this.hints.put(Hints.NAME,             this.name);
>        this.hints.put(Hints.FOO,              this.foo);
>        this.hints.put(Hints.AN_OTHER_FACTORY, this.anOtherFactory);
>    }
Sorry martin I understand your example ... it just does not work in this 
case.  featureFactory.add( AttributeType ) is called, and when it is 
called in two threads we have problems. The actual state - a list of 
AttributeTypes is not provided as a hint, and instead is supposed to be 
gathered up.
> The keys elements are the 3 "hints.put(key, value)" instructions at 
> the end of the above snipset. They copy the values in a hints HashMap 
> inherited from AbstractFactory. FactoryRegistry uses this information 
> in order to determine if an existing Factory match the requested hints.
>
> In summary:
>   * Do not tell FactoryRegistry to not cache. Let him cache.
>   * Be sure to gives to AbstractFactory.hints every hints that your 
> factory
>     uses. Do not copy the whole "userHints" map; only the hints that 
> are really
>     used with the values that are actually used (this is why the above 
> code put
>     the hints after the default values have been taken in account).
>
> Then everything should work well...
>> FeatureTypeBuilder asks CommonFactoryFinder for an instance of 
>> FeatureTypeFactory. Right now we will just create one using "new" ... 
>> unless someone can tell me how to turn off FactoryRegistry caching.
> See above. Do not turn it off. It should work fine if 
> AbstractFactory.hints has the right info.
Cannot do it ... made it return a new instance each time.
Jody

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to