I've looked at your changes--I like them, and I'm
thankful to have someone on our team to be able to
redesign the properties as you have.  Getting rid of
the 250 autogenerated or so classes will be a welcome

Comments right now:

1.)  Unlike what I was saying earlier, I don't think
we should move from Property.Maker to a new
PropertyMaker class after all, your design looks fine.
 I've noticed most subclasses of Property.Maker are
within subclasses of Properties themselves (e.g.,
LengthProperty, LengthProperty.Maker, etc.) so it
looks like a neat, clean design.

2.)  The new class appears (1)
autogenerated, and (2) to be an XSLT masterpiece at
that as well.  If it is indeed in good shape, I'd like
you to submit it to Bugzilla as the new
fo-property-mapping.xsl, replacing the old one of that
name in src/codegen.  (We won't apply it however,
until we no longer need the current autogenerated
fo-property-mapping.xsl, i.e., until all the old
properties have been tossed out.)

This way if we have to make wide-ranging changes to
FOPropertyMapping, we'll have a XSLT source file we
can conveniently work with.  (Note that putting it in
codegen does *not* mean that it will be automatically
autogenerated anymore--it won't, just as constants.xsl
no longer is--we'll pull it out of the main Ant build
target at that time and keep it the separate, manual
xsltToJava target in our build file[1].


Comments on FOPropertyMapping:

I like removing all these autogenerated classes, but I
think we can still keep some processing at
compile-time for more of a performance gain, as

3)  I think the runtime construction of the generic
properties (genericColor, genericCondBorderWidth,
etc.) may not be necessary.  We can still have those
xslt-generated into classes (6-8 classes total), but
this time we check them into FOP (again, keeping the
xsl available for manual re-generation when needed). 
But most of the generic classes are so small (your
initialization of GenericCondPadding is only 4 lines
of code), that going back to creating concrete classes
would be noticeably beneficial either, so I'm not
recommending this change.

One thing that *does* stick out, however, is the 100
or so addKeyword() calls for genericColor (the largest
of the generic properties):

  genericColor.addKeyword("antiquewhite", "#faebd7");
  genericColor.addKeyword("aqua", "#00ffff");
  genericColor.addKeyword("aquamarine", "#7fffd4");

I'd like us to have a static array of these
values--i.e., something done compile-time, that
genericColor can just reference, so we don't have to
do this keyword initialization.  

4)  I'd also like us to, rather than call
setInherited() and setDefault() for each of the
properties during initialization, for the
Property/Property.Maker classes to just reference that
information from two (new) static arrays, added to  We can also get rid of these two
setter methods as well (ideally there shouldn't be
setters for these attributes anyway--they should
remain inherent to the Property.)

This change will allow us to take advantage of the
fact that we are now on int-constants. 
getDefault(PR_WHATEVER), for example, is just

5)  Similar to (b) above, several of the makers also
have a "useGeneric()" initialization requirement:

m  = new CondLengthProperty.Maker(PR_PADDING_END);

For those Makers that require it, I'd like the
constructor to be expanded to this:

m  = new CondLengthProperty.Maker(PR_PADDING_END,

Again, getting rid of the useGeneric() function.  This
is for more speed, encapsulation, and also shrinking
FOPropertyMapping class a bit.

Sorry for the long post.  I'll probably have other
comments in other areas, but this is all I've studied
for now.



Do you Yahoo!?
Yahoo! Hotjobs: Enter the "Signing Bonus" Sweepstakes

Reply via email to