On Mon, Jan 26, 2004 at 11:32:22AM -0000, [EMAIL PROTECTED] wrote:
>
> This patch is intended as inspiration and as an example of the discussion found
> here:
>
> http://marc.theaimsgroup.com/?l=fop-dev&m=107511296910230&w=2
>
> The patch includes the following:
>
> - unnests the Property.Maker classes.
> - moves the PropertyMakers into properties
> - Rolls the datatypes into the property classes.
> - Moves the property classes into datatypes.
I have the following considerations:
1. The old situation has pure datatypes, which in theory may be reused
in other situations. In practice, these datatypes are very much
bound to properties, so that reuse is not realistic, and does not
happen in FOP code. Combining the notions of datatype and property
is more tuned to FOP's situation.
2. Even in the old situation the separation between datatypes and
properties is not complete. Compound datatypes contain properties.
3. In code where the datatype aspect is used, the code may become less
logical. This happens in the parsers and in the RTF renderer. An
example from render/rtf/TextAttributesConverter.java:
// Cell background color
ColorTypeProperty colorTypeProp =
(ColorTypeProperty)propertyList.get(Constants.PR_COLOR);
if (colorTypeProp != null) {
ColorTypeProperty colorType = colorTypeProp.getColorType();
if (colorType != null) {
if (colorType.getAlpha() != 0
|| colorType.getRed() != 0
|| colorType.getGreen() != 0
|| colorType.getBlue() != 0) {
rtfAttr.set(
RtfText.ATTR_FONT_COLOR,
convertFOPColorToRTF(colorType));
}
Here colorTypeProp and colorType denote the same, and the code is
not quite logical. That is because the method getColorType now more
acts as an assertion. In the new situation it could be made more
logical as follows:
// Cell background color
ColorTypeProperty colorType =
(ColorTypeProperty)propertyList.get(Constants.PR_COLOR);
if (colorType != null && colorType.getColorType() != null) {
if (colorType.getAlpha() != 0
|| colorType.getRed() != 0
|| colorType.getGreen() != 0
|| colorType.getBlue() != 0) {
rtfAttr.set(
RtfText.ATTR_FONT_COLOR,
convertFOPColorToRTF(colorType));
}
Therefore, in the new situation people might want to do some
rewriting.
All in all I think that this change simplifies the code, and would be
a good change.
Allow me to make some notes:
1. Would it not be a good idea to move Property.java from fo to
properties?
2. TableColLength and LinearCombinationLength do not have the word
'Property' in their name. I would advocate making these names
consistent with the others.
3. Should ToBeImplemented.java also be removed?
A lot of good work.
Regards,
Simon Pepping
--
Simon Pepping
home page: http://www.leverkruid.nl
