You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2009/08/05 02:08:12 UTC

svn commit: r801035 - in /commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl: Interpreter.java JexlArithmetic.java

Author: sebb
Date: Wed Aug  5 00:08:11 2009
New Revision: 801035

URL: http://svn.apache.org/viewvc?rev=801035&view=rev
Log:
Make boxing explicit (use Integer.valueOf() for numbers that are likely to be small)
Remove Long=>long conversions where the return value is Long anyway!

Modified:
    commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java
    commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java

Modified: commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java?rev=801035&r1=801034&r2=801035&view=diff
==============================================================================
--- commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java (original)
+++ commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java Wed Aug  5 00:08:11 2009
@@ -371,7 +371,7 @@
             long l = arithmetic.toLong(left);
             n = 1;
             long r = arithmetic.toLong(right);
-            return l & r;
+            return new Long(l & r);
         } catch (RuntimeException xrt) {
             throw new JexlException(node.jjtGetChild(n), "long coercion error", xrt);
         }
@@ -382,7 +382,7 @@
         Object left = node.jjtGetChild(0).jjtAccept(this, data);
         try {
             long l = arithmetic.toLong(left);
-            return ~l;
+            return new Long(~l);
         } catch (RuntimeException xrt) {
             throw new JexlException(node.jjtGetChild(0), "long coercion error", xrt);
         }
@@ -398,7 +398,7 @@
             long l = arithmetic.toLong(left);
             n = 1;
             long r = arithmetic.toLong(right);
-            return l | r;
+            return new Long(l | r);
         } catch (RuntimeException xrt) {
             throw new JexlException(node.jjtGetChild(n), "long coercion error", xrt);
         }
@@ -414,7 +414,7 @@
             long l = arithmetic.toLong(left);
             n = 1;
             long r = arithmetic.toLong(right);
-            return l ^ r;
+            return new Long(l ^ r);
         } catch (RuntimeException xrt) {
             throw new JexlException(node.jjtGetChild(n), "long coercion error", xrt);
         }
@@ -438,7 +438,7 @@
             return arithmetic.divide(left, right);
         } catch (RuntimeException xrt) {
             if (!strict && xrt instanceof ArithmeticException) {
-                return 0.0;
+                return new Double(0.0);
             }
             JexlNode xnode = findNullOperand(xrt, node, left, right);
             throw new JexlException(xnode, "divide error", xrt);
@@ -879,7 +879,7 @@
             return arithmetic.mod(left, right);
         } catch (RuntimeException xrt) {
             if (!strict && xrt instanceof ArithmeticException) {
-                return 0.0;
+                return new Double(0.0);
             }
             JexlNode xnode = findNullOperand(xrt, node, left, right);
             throw new JexlException(xnode, "% error", xrt);
@@ -1003,12 +1003,12 @@
             throw new JexlException(node, "size() : argument is null", null);
         }
 
-        return sizeOf(node, val);
+        return Integer.valueOf(sizeOf(node, val));
     }
 
     /** {@inheritDoc} */
     public Object visit(ASTSizeMethod node, Object data) {
-        return sizeOf(node, data);
+        return Integer.valueOf(sizeOf(node, data));
     }
 
     /** {@inheritDoc} */

Modified: commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java?rev=801035&r1=801034&r2=801035&view=diff
==============================================================================
--- commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java (original)
+++ commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java Wed Aug  5 00:08:11 2009
@@ -66,7 +66,7 @@
      * @return Long(0)
      */
     protected Object controlNullNullOperands() {
-        return strict? null : 0L;
+        return strict? null : Long.valueOf(0);
     }
 
     /**
@@ -101,7 +101,7 @@
             if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
                 double l = toDouble(left);
                 double r = toDouble(right);
-                return l + r;
+                return new Double(l + r);
             }
         
             // if both are bigintegers use that type
@@ -124,7 +124,7 @@
             BigInteger result = l.add(r);
             if (result.compareTo(BIGI_LONG_MAX_VALUE) <= 0
                 && result.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
-                return result.longValue();
+                return result;
             }
             return result;
         } catch (java.lang.NumberFormatException nfe) {
@@ -167,7 +167,7 @@
         if (r == 0.0) {
             throw new ArithmeticException("/");
         }
-        return l / r;
+        return new Double(l / r);
 
     }
     
@@ -192,7 +192,7 @@
             if (r == 0.0) {
                 throw new ArithmeticException("/");
             }
-            return l % r;
+            return new Double(l % r);
         }
 
         // if both are bigintegers use that type
@@ -218,7 +218,7 @@
         BigInteger result = l.mod(r);
         if (result.compareTo(BIGI_LONG_MAX_VALUE) <= 0
             && result.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
-            return result.longValue();
+            return result;
         }
         return result;
     }
@@ -245,7 +245,7 @@
         if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
             double l = toDouble(left);
             double r = toDouble(right);
-            return l * r;
+            return new Double(l * r);
         }
         
         // if both are bigintegers use that type
@@ -268,7 +268,7 @@
         BigInteger result = l.multiply(r);
         if (result.compareTo(BIGI_LONG_MAX_VALUE) <= 0
             && result.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
-            return result.longValue();
+            return result;
         }
         return result;
     }
@@ -295,7 +295,7 @@
         if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
             double l = toDouble(left);
             double r = toDouble(right);
-            return l - r;
+            return new Double(l - r);
         }
         
         // if both are bigintegers use that type
@@ -318,7 +318,7 @@
         BigInteger result = l.subtract(r);
         if (result.compareTo(BIGI_LONG_MAX_VALUE) <= 0
             && result.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
-            return result.longValue();
+            return result;
         }
         return result;
     }



Re: svn commit: r801035 - in /commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl: Interpreter.java JexlArithmetic.java

Posted by sebb <se...@gmail.com>.
On 05/08/2009, Henrib <hb...@gmail.com> wrote:
>
>  Some return result.longValue() that were actually usefull to coerce
>  BigIntegers to long when possible have been removed in the clean-up.
>  As is, tests dont pass anymore; I'm adding those back in JEXL-68.

Oops, sorry about that, I thought I tested the changes.

I suggest comments are added to explain why the code looks odd.

> --
>  View this message in context: http://www.nabble.com/svn-commit%3A-r801035---in--commons-proper-jexl-branches-2.0-src-main-java-org-apache-commons-jexl%3A-Interpreter.java-JexlArithmetic.java-tp24818432p24819369.html
>
> Sent from the Commons - Dev mailing list archive at Nabble.com.
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r801035 - in /commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl: Interpreter.java JexlArithmetic.java

Posted by Henrib <hb...@gmail.com>.
Some return result.longValue() that were actually usefull to coerce
BigIntegers to long when possible have been removed in the clean-up.
As is, tests dont pass anymore; I'm adding those back in JEXL-68.
-- 
View this message in context: http://www.nabble.com/svn-commit%3A-r801035---in--commons-proper-jexl-branches-2.0-src-main-java-org-apache-commons-jexl%3A-Interpreter.java-JexlArithmetic.java-tp24818432p24819369.html
Sent from the Commons - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r801035 - in /commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl: Interpreter.java JexlArithmetic.java

Posted by Henrib <hb...@gmail.com>.
Ok, good to know; thanks for the explanation.

-- 
View this message in context: http://www.nabble.com/svn-commit%3A-r801035---in--commons-proper-jexl-branches-2.0-src-main-java-org-apache-commons-jexl%3A-Interpreter.java-JexlArithmetic.java-tp24818432p24819227.html
Sent from the Commons - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r801035 - in /commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl: Interpreter.java JexlArithmetic.java

Posted by sebb <se...@gmail.com>.
On 05/08/2009, Henrib <hb...@gmail.com> wrote:
>
>
>
>  sebb-3 wrote:
>  >
>  > Author: sebb
>  > Date: Wed Aug  5 00:08:11 2009
>  > New Revision: 801035
>  >
>  > URL: http://svn.apache.org/viewvc?rev=801035&view=rev
>  > Log:
>  > Make boxing explicit (use Integer.valueOf() for numbers that are likely to
>  > be small)
>  > Remove Long=>long conversions where the return value is Long anyway!
>  >
>  > Modified:
>  >
>  > commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java
>  >
>  > commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java
>  >
>  >
>
>
> The findbugs (maven plugin version) was flagging usage of new Long(...) as a
>  performance warning so I changed them to autoboxing which it seemed happy
>  with sometime ago.
>  Not sure if these have really an impact...
>

I think Findbugs is wrong about new Long() unless it is a small number.
[Henri Yandell and I did some tests a while back]

Probably best to suppress this warning. Maybe for Integer as well.

But Byte.valueOf() is definitely likely to be more efficient.

It appears that autoboxing uses Long.valueOf() (at least with Sun Java
1.5), which is why Findbugs did not complain (it looks at the compiled
code).

I added the new Double() etc. to various return statements because IMO
it is better to be explicit about the type that is being returned.

>  --
>  View this message in context: http://www.nabble.com/svn-commit%3A-r801035---in--commons-proper-jexl-branches-2.0-src-main-java-org-apache-commons-jexl%3A-Interpreter.java-JexlArithmetic.java-tp24818432p24818509.html
>  Sent from the Commons - Dev mailing list archive at Nabble.com.
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r801035 - in /commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl: Interpreter.java JexlArithmetic.java

Posted by Henrib <hb...@gmail.com>.


sebb-3 wrote:
> 
> Author: sebb
> Date: Wed Aug  5 00:08:11 2009
> New Revision: 801035
> 
> URL: http://svn.apache.org/viewvc?rev=801035&view=rev
> Log:
> Make boxing explicit (use Integer.valueOf() for numbers that are likely to
> be small)
> Remove Long=>long conversions where the return value is Long anyway!
> 
> Modified:
>    
> commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/Interpreter.java
>    
> commons/proper/jexl/branches/2.0/src/main/java/org/apache/commons/jexl/JexlArithmetic.java
> 
> 

The findbugs (maven plugin version) was flagging usage of new Long(...) as a
performance warning so I changed them to autoboxing which it seemed happy
with sometime ago.
Not sure if these have really an impact...

-- 
View this message in context: http://www.nabble.com/svn-commit%3A-r801035---in--commons-proper-jexl-branches-2.0-src-main-java-org-apache-commons-jexl%3A-Interpreter.java-JexlArithmetic.java-tp24818432p24818509.html
Sent from the Commons - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org