Re: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-02-02 Thread Glen Mazza
--- Finn Bock <[EMAIL PROTECTED]> wrote:
> I like it. How does this sound:
> 
> - Unnest Property.Maker to PropertyMaker and put it
> in fo.properties.
> - Roll the datatypes classes into their property
> class and move the
>property class to fo.properties (but without
> unnesting their Maker
>class because the remaining nested Maker classes
> are really trivial).
> - Move the handcoded maker classes to fo.properties.
> 
> regards,
> finn
> 

Fortrinlig Tak nemlig jeres arbejde her ovre!
(= Excellent!  Thanks for your work here.)

Glen (who recommends we stay with English, despite the
availability of machine translation... ;)


__
Do you Yahoo!?
Yahoo! SiteBuilder - Free web site building tool. Try it!
http://webhosting.yahoo.com/ps/sb/


DO NOT REPLY [Bug 26434] - [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-02-02 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434

[PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED



--- Additional Comments From [EMAIL PROTECTED]  2004-02-02 16:12 ---
Applied, but with the comments from fop-dev. Main difference from the patch is:

- Only the Property.Maker is unnested.
- The property classes are moved to fop.fo.properties.


DO NOT REPLY [Bug 26434] - [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-02-02 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434

[PATCH] unnesting Property.Maker and rollling datatypes into thier properties.





--- Additional Comments From [EMAIL PROTECTED]  2004-02-02 08:24 ---
Comments from fop-dev:

http://marc.theaimsgroup.com/?l=fop-dev&m=107476581612018&w=2
http://marc.theaimsgroup.com/?l=fop-dev&m=107557068607807&w=2


Re: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-02-01 Thread Finn Bock
[Glen Mazza]

Another option, Finn, is to move all the Property 
subclasses to fo.properties (even if they're alongside
the makers, nested or unnested), after thinking about
it, I think that will be a little bit clearer than
having them in the datatype package.  Comments?
I like it. How does this sound:

- Unnest Property.Maker to PropertyMaker and put it in fo.properties.
- Roll the datatypes classes into their property class and move the
  property class to fo.properties (but without unnesting their Maker
  class because the remaining nested Maker classes are really trivial).
- Move the handcoded maker classes to fo.properties.
regards,
finn


Re: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-02-01 Thread Finn Bock
[Simon Pepping]

I have the following considerations:
...
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));
}
Yes, I hadn't though of that, perhaps because such constructs are rare 
in layoutmgr and area renderers. But I think it should be rewritten to:

// Cell background color
ColorTypeProperty colorType =
   propertyList.get(Constants.PR_COLOR).getColorType();
if (colorType.getAlpha() != 0
|| colorType.getRed() != 0
|| colorType.getGreen() != 0
|| colorType.getBlue() != 0) {
rtfAttr.set(RtfText.ATTR_FONT_COLOR,
convertFOPColorToRTF(colorType));
}
without the cast and without testing for a null value because there is 
*always* some computed value for PR_COLOR and it is *always* a 
colorType. Oh, and the comment should be fixed as well .

All in all I think that this change simplifies the code, and would be
a good change.
Thanks.

Allow me to make some notes:

1. Would it not be a good idea to move Property.java from fo to
   properties?
Yes, I agree with you and Glen on this.

2. TableColLength and LinearCombinationLength do not have the word
   'Property' in their name. I would advocate making these names
   consistent with the others.
Perhaps, I don't have strong feelings about naming.

3. Should ToBeImplemented.java also be removed?

A lot of good work.
Thanks, but eclipse did all the work for me. It's a great tool for this 
kind of refactoring.

regards,
finn


Re: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-01-31 Thread Peter B. West
Simon Pepping wrote:
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.
Alt-design has completely separate properties and data-types.  Instances 
of datatypes contain int references to the property on which they were 
defined.
2. Even in the old situation the separation between datatypes and
   properties is not complete. Compound datatypes contain properties.
Alt-design has no compound properties.

Peter
--
Peter B. West 


RE: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-01-31 Thread Glen Mazza

--- "Andreas L. Delmelle" <[EMAIL PROTECTED]>
wrote:
> > -Original Message-
> > From: Simon Pepping
> [mailto:[EMAIL PROTECTED]
> >
> 
> > 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?
> >
> 
> A question that was on the tip of my tongue too...
> I'd think: not only
> Property.java, but all related Maker-classes as
> well.
> 

Another option, Finn, is to move all the Property 
subclasses to fo.properties (even if they're alongside
the makers, nested or unnested), after thinking about
it, I think that will be a little bit clearer than
having them in the datatype package.  Comments?

Thanks,
Glen


__
Do you Yahoo!?
Yahoo! SiteBuilder - Free web site building tool. Try it!
http://webhosting.yahoo.com/ps/sb/


RE: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-01-31 Thread Andreas L. Delmelle
> -Original Message-
> From: Simon Pepping [mailto:[EMAIL PROTECTED]
>

> 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?
>

A question that was on the tip of my tongue too... I'd think: not only
Property.java, but all related Maker-classes as well.


> 3. Should ToBeImplemented.java also be removed?

In the long run, that *should* be the objective indeed. For the moment, I'd
leave that right where it is, until we're sure we've no more refs to it
hidden somewhere else in the code.


Cheers,

Andreas



Re: [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-01-31 Thread Simon Pepping
On Mon, Jan 26, 2004 at 11:32:22AM -, [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



DO NOT REPLY [Bug 26434] - [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-01-26 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434

[PATCH] unnesting Property.Maker and rollling datatypes into thier properties.





--- Additional Comments From [EMAIL PROTECTED]  2004-01-26 11:33 ---
Created an attachment (id=10086)
A unified patch against HEAD.


DO NOT REPLY [Bug 26434] New: - [PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

2004-01-26 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26434

[PATCH] unnesting Property.Maker and rollling datatypes into thier properties.

   Summary: [PATCH] unnesting Property.Maker and rollling datatypes
    into thier properties.
   Product: Fop
   Version: 1.0dev
  Platform: Other
OS/Version: Other
Status: NEW
  Severity: Normal
  Priority: Other
 Component: general
AssignedTo: [EMAIL PROTECTED]
ReportedBy: [EMAIL PROTECTED]


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.

The patch moves files around into new packages but I have not been able to get 
a 'delete file' into the patch, so there is some files that must be manually 
deleted after applying the patch:

src/java/org/apache/fop/datatypes/AutoLength.java
src/java/org/apache/fop/datatypes/ColorType.java
src/java/org/apache/fop/datatypes/CondLength.java
src/java/org/apache/fop/datatypes/FixedLength.java
src/java/org/apache/fop/datatypes/Keep.java
src/java/org/apache/fop/datatypes/Length.java
src/java/org/apache/fop/datatypes/LengthPair.java
src/java/org/apache/fop/datatypes/LengthRange.java
src/java/org/apache/fop/datatypes/MixedLength.java
src/java/org/apache/fop/datatypes/PercentLength.java
src/java/org/apache/fop/datatypes/Space.java
src/java/org/apache/fop/fo/BorderWidthPropertyMaker.java
src/java/org/apache/fop/fo/CharacterProperty.java
src/java/org/apache/fop/fo/ColorTypeProperty.java
src/java/org/apache/fop/fo/CompoundPropertyMaker.java
src/java/org/apache/fop/fo/CondLengthProperty.java
src/java/org/apache/fop/fo/CorrespondingPropertyMaker.java
src/java/org/apache/fop/fo/DimensionPropertyMaker.java
src/java/org/apache/fop/fo/EnumProperty.java
src/java/org/apache/fop/fo/IndentPropertyMaker.java
src/java/org/apache/fop/fo/KeepProperty.java
src/java/org/apache/fop/fo/LengthPairProperty.java
src/java/org/apache/fop/fo/LengthProperty.java
src/java/org/apache/fop/fo/LengthRangeProperty.java
src/java/org/apache/fop/fo/LineHeightPropertyMaker.java
src/java/org/apache/fop/fo/ListProperty.java
src/java/org/apache/fop/fo/NumberProperty.java
src/java/org/apache/fop/fo/SpaceProperty.java
src/java/org/apache/fop/fo/StringProperty.java