[Simon Pepping]

I have just catched up with the massive changes to the property
system. Allow me to share a few observations:

Thanks for your comments. How do you otherwise think it compares to the previous generated property makers?


1. If I see correctly, PropertySets is not yet used.

Correct. Its intended use is when we, at some point in the future, want to store more than just the specified properties in the FObjs.


   PropertyList is
   still a HashMap keyed on property name. Is this waiting for
   some other changes to be made?

Yes. Either PropertyList should have a Property[] array indexed directly by the propId or the HashMap should be keyed by an Integer object with the same value as the propId:


super.get(integerArray[propId]);

where integerArray is initialized as:

   integerArray[1] = new Integer(1);
   integerArray[2] = new Integer(2);
   ...

Which of these it will be depends on how much information we decide to store in the Fobj.

2. In FOPropertyMapping the word 'generic' is used in two different
   senses: in s_generics and getGenericMappings() on the one hand,
   and in genericBoolean etc., createGenerics() and useGeneric() on
   the other hand. This may be confusing. One might e.g. be tempted to
   think that s_generics contains the objects genericXxx only, which
   is not the case.

You are absolutely correct. I've took the names from the code which existed at the time. I'm also terrible at naming things so other suggestions will be welcome. I suggest that we rename s_generics and getGenericMappings to s_fomapping and getFoMappings because they deal with the properties from the xsl:fo spec.


3. getGenericMappings rebuilds s_generics every time it is called. In
   the current code it seems to be called only once, by FObj when the
   class is loaded. Would it not be better if getGenericMappings
   itself would ensure that the array is built only once?

Yes, but I would like to take the question a bit further and ask where such information should be cached? Storing it in static variables caches it in the classloader which makes it difficult to control the release of the memory.


Perhaps it would be better to store the information in the Driver or FOUserAgent, IOW to put the caching under user control.

If caching at the classloader level is the best way to do it, then it makes perfect sense to do what you suggest.

regards,
finn



Reply via email to