Thanks so much for your detailed review .... I also do not mind if you edit the pages or leave a comment on them, I am mostly about collecting the information.
Additional comments inline ... > Hello Jody > > The documentation work you did seems a very good overview of the way factories > are organized in the referencing module! It may be an opportunity to see if > there is things to fix. > > There is some minor observations: > > http://docs.codehaus.org/display/GEOTDOC/0+Referencing+Overview > --------------------------------------------------------------- > There is some reference to FactoryEPSGExtra. Maybe the intend was > "EsriExtension" and "UnnamedExtension"? > I was just going by the class hierarchy as shown to me in eclipse? > At the end of the page, I suggest to replace the sentence "so they can share a > common class" by something like (sentence to be fixed as you like): > > "so the backing factory can take advantage of caching provided > by the buffer. The performance gain is considerable, because > the creation of a referencing object typically involve many > dependencies shared by previous instances. For example many > ProjectedCRS are based on the same GeographicCRS, and many > GeographicCRS are based on the same Datum." > > Note: "considerable" means at least 2 or 3 time faster. I don't have benchmark > in mind, but it was a lot and part of the performance improvement was done by > Andrea. > Sweet. > http://docs.codehaus.org/display/GEOTDOC/1+Referencing+Configuration+and+Tool > ----------------------------------------------------------------------------- > This page contains a "-DGeoTools.FORCE_LONGITUDE_FIRST_AXIS_ORDER=true" > property > defined on the command line. The current property name is rather > "org.geotools.referencing.forceXY", but we could change that. Should we take > the > former one as a proposal for adding a mechanism that allow every hints (not > just > a few ones) to be specifiable on the command line? > I already did this; it was part of hooking up GeoTools.getDefaultHints(); the default hints are populated by using reflection and checking environmental variables near as I remember. forceXY is easier to understand however; you probably do not prefer it as X and Y are not always on the table. > http://docs.codehaus.org/display/GEOTDOC/2+Referencing+Factories > ---------------------------------------------------------------- > About the use of pool in GeotoolsFactory, we could add the following > explanation: > > This pool do not provides any performance gain, since the new referencing > object > is created anyway and only then compared to existing object in the pool. If an > existing object is found, the new one is discarted. The purpose of this pool > is > rather to reduce memory usage, since a lot of referencing objects share > smaller > building blocks. For example many ProjectedCRS objects are based on the same > GeographicCRS, and many GeographicCRS use the same Datum. The same > CoordinateSystem and axises also tend to be reused often. > Understood ... similar to "intern"ing Strings. > The pool works on the basis of two assumptions: all referencing objects are > immutable, and the 'equals(Object)' method is defined in a very strict way: > the > two objects must be of exactly the same implementation class, and every > attributes - regardless their "public" or "private" visibility - are compared > field by field. Floating point values are compared for an exact match; there > is > no "tolerance" threshold, no "ignore metadata" flag. In Geotools > implementation, > two referencing objects are considered equal only if they are really identical > objects in every aspects. > > The fact that GeoAPI 2.0 do not defines the CoordinateReferenceSystem > 'equals(Object)' and 'hashCode()' contract is not an issue for this pool, > because GeotoolsFactory creates only Geotools implementations which are known > to > implement 'equals(Object)' in strict way. Current 'equals(Object)' > implementation may be more restrictive than needed, but this is on purpose in > order to preserve the reflexivity requirement in the presence of foreigner > implementations that may not implement the contract in the same way. > Got it; it makes sense - and documenting it as such will help figure out DirectPosition. > http://docs.codehaus.org/display/GEOTDOC/2+Referencing+Factories > ---------------------------------------------------------------- > A little bit lower on the same page, we can read: > > FactoryUsingAnsiSQL - with the EPSG database as shipped for Access > > This is actually FactoryUsingSQL, since MS-Access SQL dialect is not Ansi. The > FactoryUsingAnsiSQL modify the MS-Access dialect to make it Ansi-compliant. > The > MS-Access access is the base class because it is the primary format in which > the > EPSG database is maintained. Other formats like PostgreSQL are derived from > the > MS-Access database. > :-) Cool. I was going quickly - I may check the javadocs in a moment and ensure they align with this. > Jody Garnett a écrit : > >> http://docs.codehaus.org/display/GEOTDOC/3+Authorities+backed+by+the+EPSG+Database >> >> I am a bit bogged down in FactoryUsingSQL the class seems to have a >> evolved to use a large number of Maps to track or cache information >> during use. This may be a sign that some helper classes are needed (at >> least to act as a the value in a single map? >> > Maybe... Actually some of those Maps are distributed among the various super > classes, which provide some form of (maybe unsuffisient) separation of > concern. > For example 'unmodifiableHints' is a private field in AbstractFactory. > I am sure Java5 would clean up some of the opportunities for confusion as well. I will try documenting these Maps this morning and see if anything jumps out at me. > The UML in the above page lists private fields as well as public or protected > ones. Maybe it would be better to hide private fields, since they often add > complexity to the UML without real value. For example the 'hints' protected > field is significant, but most developpers probably don't need to care much > about the related 'unmodifiableHints' and 'hintsInitialized' private fields; > they are pretty low in implementation details. And more important, private > fields may change at any time and become out of phase with the documentation > if > they are documented. > I am really interested in sorting out connection issues and concurancy, so for now I would like to list the private fields. I can construct a seperate diagram (a class diagram) showing the public and protected fields. The interaction diagram is intended to show the "live" objects at runtime ... and all their memory use. I will be trying to sort out which fields are hidden behind what locking / thread lock technique. Cheers, Jody ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
