Re: [Geotools-devel] IRC Breakout - the Referencing Proposal

[Jody G. <jga...@re...>]

Martin Desruisseaux wrote:
> Jody Garnett a écrit :
>   
>> The proposal is available here:
>> -
>> http://docs.codehaus.org/display/GEOTOOLS/Improve+CRSAuthority+Concurrency+Caching+and+Connection+Use
>>     
>
>
> Just a quick note for discussion purpose:
>
> OracleEPSGDefinitionDAO
> -----------------------
> Is it a replacement for FactoryUsingSQL? 
Yes so.
> If so, the documentation said "Retrieves just the definition for the provided code in the form a Map of
> properties". This is a significant change compared to actual implementation,
> which create a full (uncached) object. 
Indeed - I can back out the change but I would like you to think about it.
> What is the purpose of this change?
The recursive nature of handling cache misses when constructing a 
compound object. There are sequence diagrams etc which cory is uploading 
now to show the problem / tradeoff.

> Who would construct the objects from the map? What would be the Map keys? I wonder
> if it would introduce yet more complexity instead of making things clearer. Just
> defining the Map keys may be non-trivial (some keys could have multiple values)
> and add to the "conceptual overhead". We would also lost type safety at least
> for the part of the code working with Map. The proposal don't said which benefit
> we would get in return.
>   
We need the Pros Cons section for the different alternatives (our 
proposal page does not facilitate discussion - only capturing the final 
design).

The reason for such a change is to reduce the worker object to just a 
data access object - no construction - just retrieve the definition from 
the database. In one of my pattern books this is called a "RowAccessor" 
... but since we have one object that is acessing many tables the 
DataAccessObject marker makes sense. This suggestion (and request for 
clarity) is by way of Martin Davis (thanks Martin).

I am basing the seperation as a Map of "properties" based on a code 
review and the sequence diagrams for creating a Datum.
> ReferencingObjectCache
> ----------------------
> It is not clear to me what would be the benifit of splitting
> BufferedAuthorityFactory into BufferedAuthorityFactory + ReferencingObjectCache.
> After all, BufferedAuthorityFactory is almost just that: a cache with
> specialized 'getXXX' method (actually called 'createXXX', but this is the same).
> And those specialized 'createXXX' methods are valuable to FactoryUsingSQL, again
> for type safety.
>   
Just a separation of concerns - several of the fields and methods are 
*only* focused on acting as a cache. Taking them out into a separate 
object helps keep things clear. The other reason
is so we can *inject* this object into the FactoryUsingOracleSQL.
> Maybe the concern is about FactoryOnOracle which extends (indirectly)
> DeferredAuthorityFactory which extends BufferedAuthorityFactory? In such case,
> rather than introducing a ReferencingObjectCache, what about revisiting the
> relationship between BufferedAuthorityFactory and DeferredAuthorityFactory?
> Maybe the later should not extends the former (I'm not sure; just droping idea
> for investigation).
Nope those super classes make sense - the problem is sharing a cache 
between many workers. You can either:
- reduce the workers to only providing the definition (in which case the 
parent will make the objects and store them away in its internal cache - 
which still should be seperate for debugging and clarity); or
- take your internal cache (seperated out as an object) and inject it 
into all the workers - so they can do their own cache lookup when 
building compound objects
> We could set DeferredAuthorityFactory as a subclass of AuthorityFactoryAdapter instead, and override
> the 'getXXXAuthorityFactory' in
> order to return one of the direct EPSG authority factory in the pool.
> BufferedAuthorityFactory would be a completly independent class, and probably a
> leaf. FactoryUsingSQL could continue to reference it directly.
>   
Separating out BufferedAuthorityFactory as a pure decorator is only 
"okay"; you still have the very difficult situation of a pool of workers 
all of which need to share the same cache. This would be a case where a 
container solution would work; the decorator would add the "cache 
object" into the container, and when a new worker was created in that 
context it could pick up the cache object if one was actually being used.

Sorry for the detail Martin - we are on a collision course between ways 
of thinking about software. The next couple of days should be fun.
> On class renaming
> -----------------
> Just for clarification about the intend of "OracleEPSGAuthority" name... To me,
> it sound like "Oracle flavor of EPSG", I means EPSG codes modified by Oracle
> (some different CRS, or addition, or removal...). The intend of
> "FactoryOnOracle" (or maybe EpsgOnOracle would be a better name) was to means
> "strict (admitly unmodified) EPSG database hosted on a Oracle database
> software", which doesn't sound like the same than "Oracle EPSG" to me. But I'm
> not an English native speaker, so I can tell for sure...
>
> What do you think?
>   
I will just change it as you request - I am going to get hung up on the 
details of the names (only that they reflect the software pattern they 
are involved in).

Jody