You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by ad...@apache.org on 2007/07/20 18:48:57 UTC

svn commit: r558045 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties: EnumNumber.java EnumProperty.java NumberProperty.java StringProperty.java

Author: adelmelle
Date: Fri Jul 20 09:48:55 2007
New Revision: 558045

URL: http://svn.apache.org/viewvc?view=rev&rev=558045
Log:
Slight correction: 
- make NumberProperty, EnumProperty, EnumNumber and StringProperty final, so instanceof suffices in the check for equality
- instead of subclassing NumberProperty, make EnumNumber implement the Numeric interface


Modified:
    xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumNumber.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumProperty.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/NumberProperty.java
    xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/StringProperty.java

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumNumber.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumNumber.java?view=diff&rev=558045&r1=558044&r2=558045
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumNumber.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumNumber.java Fri Jul 20 09:48:55 2007
@@ -19,10 +19,14 @@
  
 package org.apache.fop.fo.properties;
 
+import org.apache.fop.datatypes.Numeric;
+import org.apache.fop.datatypes.PercentBaseContext;
+import org.apache.fop.fo.expr.PropertyException;
+
 /**
  * A number quantity in XSL which is specified as an enum, such as "no-limit".
  */
-public class EnumNumber extends NumberProperty {
+public final class EnumNumber extends Property implements Numeric {
 
     /** cache holding all canonical EnumNumber instances */
     private static final PropertyCache cache = new PropertyCache();
@@ -34,13 +38,13 @@
      * @param enumProperty  the base EnumProperty
      */
     private EnumNumber(Property enumProperty) {
-        super(null);
         this.enumProperty = (EnumProperty) enumProperty;
     }
 
     /**
      * Returns the canonical EnumNumber instance corresponding
      * to the given Property
+     * 
      * @param enumProperty  the base EnumProperty
      * @return  the canonical instance
      */
@@ -49,45 +53,22 @@
                 new EnumNumber((EnumProperty) enumProperty));
     }
 
+    /** {@inheritDoc} */
     public int getEnum() {
         return enumProperty.getEnum();
     }
 
-    /**
-     * Returns the length in 1/1000ths of a point (millipoints)
-     * @return the length in millipoints
-     */
-    public int getValue() {
-        log.error("getValue() called on " + enumProperty + " number");
-        return 0;
-    }
-
-    /**
-     * Returns the value as numeric.
-     * @return the length in millipoints
-     */
-    public double getNumericValue() {
-        log.error("getNumericValue() called on " + enumProperty + " number");
-        return 0;
-    }
-
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public String getString() {
         return enumProperty.toString();
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public Object getObject() {
         return enumProperty.getObject();
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public boolean equals(Object obj) {
         if (obj instanceof EnumNumber) {
             return (((EnumNumber)obj).enumProperty == this.enumProperty);
@@ -96,11 +77,65 @@
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public int hashCode() {
         return enumProperty.hashCode();
     }
+
+    /** {@inheritDoc} */
+    public int getDimension() {
+        return 0;
+    }
+
+    /** 
+     * {@inheritDoc}
+     * Always <code>true</code> for instances of this type
+     */
+    public boolean isAbsolute() {
+        return true;
+    }
     
+    /** 
+     * {@inheritDoc} 
+     * logs an error, because it's not supposed to be called
+     */
+    public double getNumericValue(PercentBaseContext context) throws PropertyException {
+        log.error("getNumericValue() called on " + enumProperty + " number");
+        return 0;
+    }
+
+    /** 
+     * {@inheritDoc} 
+     * logs an error, because it's not supposed to be called
+     */
+    public int getValue(PercentBaseContext context) {
+        log.error("getValue() called on " + enumProperty + " number");
+        return 0;
+    }
+
+    /** 
+     * {@inheritDoc} 
+     * logs an error, because it's not supposed to be called
+     */
+    public int getValue() {
+        log.error("getValue() called on " + enumProperty + " number");
+        return 0;
+    }
+
+    /** 
+     * {@inheritDoc} 
+     * logs an error, because it's not supposed to be called
+     */
+    public double getNumericValue() {
+        log.error("getNumericValue() called on " + enumProperty + " number");
+        return 0;
+    }
+
+    /** 
+     * {@inheritDoc}
+     */
+    public Numeric getNumeric() {
+        return this;
+    }
+
 }

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumProperty.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumProperty.java?view=diff&rev=558045&r1=558044&r2=558045
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumProperty.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/EnumProperty.java Fri Jul 20 09:48:55 2007
@@ -26,7 +26,7 @@
 /**
  * Superclass for properties that wrap an enumeration value
  */
-public class EnumProperty extends Property {
+public final class EnumProperty extends Property {
     
     /** cache holding all canonical EnumProperty instances */
     private static final PropertyCache cache = new PropertyCache();

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/NumberProperty.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/NumberProperty.java?view=diff&rev=558045&r1=558044&r2=558045
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/NumberProperty.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/NumberProperty.java Fri Jul 20 09:48:55 2007
@@ -32,7 +32,7 @@
 /**
  * Class for handling numeric properties
  */
-public class NumberProperty extends Property implements Numeric {
+public final class NumberProperty extends Property implements Numeric {
 
     /**
      * Inner class for making NumberProperty objects
@@ -77,7 +77,7 @@
      * Constructor for Number input
      * @param num Number object value for property
      */
-    protected NumberProperty(Number num) {
+    private NumberProperty(Number num) {
         this.number = num;
     }
 
@@ -224,13 +224,15 @@
     
     /** {@inheritDoc} */
     public boolean equals(Object o) {
-        if (o != null && o instanceof NumberProperty) {
+        if (o == this) {
+            return true;
+        }
+        if (o instanceof NumberProperty) {
             NumberProperty np = (NumberProperty) o;
             return (np.number == this.number
                     || (this.number != null
                         && this.number.equals(np.number)));
-        } else {
-            return false;
         }
+        return false;
     }
 }

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/StringProperty.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/StringProperty.java?view=diff&rev=558045&r1=558044&r2=558045
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/StringProperty.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties/StringProperty.java Fri Jul 20 09:48:55 2007
@@ -26,7 +26,7 @@
  * Exists primarily as a container for its Maker inner class, which is
  * extended by many string-based FO property classes.
  */
-public class StringProperty extends Property {
+public final class StringProperty extends Property {
 
     /**
      * Inner class for making instances of StringProperty



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: svn commit: r558045 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties: EnumNumber.java EnumProperty.java NumberProperty.java StringProperty.java

Posted by Andreas L Delmelle <a_...@pandora.be>.
On Aug 11, 2007, at 09:17, Vincent Hennebert wrote:

>> <snip />
>> Good idea, much better than the above! Didn't quite occur to me to do
>> that. Then again, in case you didn't notice: those lines are  
>> removed...(?)
>
> Ok, I isolated the wrong snippet... Go below in the commit message,  
> and
> you’ll find plenty of such lines with plus instead of minus :-P

Yeah, I knew that... ;-)

> <snip/>
>>> All that said... if most methods of the Numeric interface aren’t
>>> applicable to EnumNumber, should that class still be considered as
>>> a Numeric object? Does that make sense to cast an EnumNumber into
>>> a Numeric?
>>
>> Well, apparently, a long time ago, someone felt it necessary to  
>> have a
>> Property type that stored an enum but in the end it's only a number
>> (values like "no-limit").
>> The idea of an EnumNumber itself always seemed somewhat ugly to  
>> me, but
>> I never took the time to come up with a decent alternative.
>
> So we can consider that this class will eventually be removed/changed?
> Then this hack is more acceptable. What about a “TODO this is ugly and
> shall be removed”?

Also a fine suggestion. Now that the point has been raised, I'm  
trying to assess why it currently needs to exist in the first place.  
Maybe we could even do without, which would be even better. It just  
looked a little like EnumNumber is an artificial construction to have  
an Enum that can act as a Numeric (?), and in that case it seemed  
cleaner to me to implement the Numeric interface than have it extend  
NumberProperty just to override those methods that aren't supposed to  
be called anyway...

EnumLength is another one of those hybrids, BTW.


Cheers

Andreas

Re: svn commit: r558045 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties: EnumNumber.java EnumProperty.java NumberProperty.java StringProperty.java

Posted by Vincent Hennebert <vi...@anyware-tech.com>.
Hi Andreas,

Andreas L Delmelle a écrit :
> On Aug 8, 2007, at 18:30, Vincent Hennebert wrote:
>
>>> -    public int getValue() {
>>> -        log.error("getValue() called on " + enumProperty + " number");
>>> -        return 0;
>>
>> That may be discussed, but I have strong feelings against that. If the
>> method shouldn’t be called, why not throw an IllegalStateException or
>> so?
>
> Good idea, much better than the above! Didn't quite occur to me to do
> that. Then again, in case you didn't notice: those lines are removed...(?)

Ok, I isolated the wrong snippet... Go below in the commit message, and
you’ll find plenty of such lines with plus instead of minus :-P

<snip/>
>> All that said... if most methods of the Numeric interface aren’t
>> applicable to EnumNumber, should that class still be considered as
>> a Numeric object? Does that make sense to cast an EnumNumber into
>> a Numeric?
>
> Well, apparently, a long time ago, someone felt it necessary to have a
> Property type that stored an enum but in the end it's only a number
> (values like "no-limit").
> The idea of an EnumNumber itself always seemed somewhat ugly to me, but
> I never took the time to come up with a decent alternative.

So we can consider that this class will eventually be removed/changed?
Then this hack is more acceptable. What about a “TODO this is ugly and
shall be removed”?

Cheers,
Vincent


Re: svn commit: r558045 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties: EnumNumber.java EnumProperty.java NumberProperty.java StringProperty.java

Posted by Andreas L Delmelle <a_...@pandora.be>.
On Aug 8, 2007, at 18:30, Vincent Hennebert wrote:

>> -    public int getValue() {
>> -        log.error("getValue() called on " + enumProperty + "  
>> number");
>> -        return 0;
>
> That may be discussed, but I have strong feelings against that. If the
> method shouldn’t be called, why not throw an IllegalStateException or
> so?

Good idea, much better than the above! Didn't quite occur to me to do  
that. Then again, in case you didn't notice: those lines are  
removed...(?)

> If it is called that means there’s a bug somewhere. Perhaps the code
> won’t crash, perhaps nothing will even be noticeable on the output,  
> but
> I doubt it as there can only be a serious problem, that should be
> corrected sooner rather than later.
>
> All that said... if most methods of the Numeric interface aren’t
> applicable to EnumNumber, should that class still be considered as
> a Numeric object? Does that make sense to cast an EnumNumber into
> a Numeric?

Well, apparently, a long time ago, someone felt it necessary to have  
a Property type that stored an enum but in the end it's only a number  
(values like "no-limit").
The idea of an EnumNumber itself always seemed somewhat ugly to me,  
but I never took the time to come up with a decent alternative.


Cheers

Andreas




Re: svn commit: r558045 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/properties: EnumNumber.java EnumProperty.java NumberProperty.java StringProperty.java

Posted by Vincent Hennebert <vi...@anyware-tech.com>.
Hi Andreas,

Sorry for a late comment on this.

> Author: adelmelle
> Date: Fri Jul 20 09:48:55 2007
> New Revision: 558045
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=558045
> Log:
> Slight correction: 
> - make NumberProperty, EnumProperty, EnumNumber and StringProperty final, so instanceof suffices in the check for equality
> - instead of subclassing NumberProperty, make EnumNumber implement the Numeric interface

<snip/>
> -    public int getValue() {
> -        log.error("getValue() called on " + enumProperty + " number");
> -        return 0;

That may be discussed, but I have strong feelings against that. If the 
method shouldn’t be called, why not throw an IllegalStateException or 
so? If it is called that means there’s a bug somewhere. Perhaps the code 
won’t crash, perhaps nothing will even be noticeable on the output, but 
I doubt it as there can only be a serious problem, that should be 
corrected sooner rather than later.

All that said... if most methods of the Numeric interface aren’t 
applicable to EnumNumber, should that class still be considered as 
a Numeric object? Does that make sense to cast an EnumNumber into 
a Numeric?

WDYT?
Vincent