You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/04/08 23:25:04 UTC

svn commit: r646076 - /tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java

Author: markt
Date: Tue Apr  8 14:25:04 2008
New Revision: 646076

URL: http://svn.apache.org/viewvc?rev=646076&view=rev
Log:
This fixes 44766 but I can't see why the code was written as originally coded. Any light much appreciated. I'll give it a couple of days and then, assuming there are no objections, propose it for 6.0.x.

Modified:
    tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java

Modified: tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java?rev=646076&r1=646075&r2=646076&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java (original)
+++ tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java Tue Apr  8 14:25:04 2008
@@ -326,8 +326,8 @@
         return (obj != null && isNumberType(obj.getClass()));
     }
 
-    public final static boolean isNumberType(final Class type) {
-        return type == (java.lang.Long.class) || type == Long.TYPE || type == (java.lang.Double.class) || type == Double.TYPE || type == (java.lang.Byte.class) || type == Byte.TYPE || type == (java.lang.Short.class) || type == Short.TYPE || type == (java.lang.Integer.class) || type == Integer.TYPE || type == (java.lang.Float.class) || type == Float.TYPE || type == (java.math.BigInteger.class) || type == (java.math.BigDecimal.class);
+    public final static boolean isNumberType(final Class<?> type) {
+        return (Number.class.isAssignableFrom(type));
     }
 
     /**



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


Re: svn commit: r646076 - /tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java

Posted by Mark Thomas <ma...@apache.org>.
Konstantin Kolinko wrote:
> I've created a patch to clean them:
> https://issues.apache.org/bugzilla/attachment.cgi?id=21797

Many thanks.

> What is the history of this code before rev. 389146 of 2006-03-27 when
> /tomcat/tc6.0.x/trunk was created?

As I recall, the whole EL implementation was provided as a single 
contribution and Remy included it when he created the 6.0.x branch.

Mark


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


Re: svn commit: r646076 - /tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
Surely, there are irregularities.

There are many places where java.lang.Class instance is not a
parameter to the method, but obtained in place by calling
obj.getClass().  In such cases comparing object's class against
primitive type classes is meaningless.

I've created a patch to clean them:
https://issues.apache.org/bugzilla/attachment.cgi?id=21797

Comparing against primitive types is only meaningful in method
isNumberType(Class) of ELArithmetic and several methods in ELSupport,
where the Class argument is passed explicitly.


What is the history of this code before rev. 389146 of 2006-03-27 when
/tomcat/tc6.0.x/trunk was created?


2008/4/9 Mark Thomas <ma...@apache.org>:
> Konstantin Kolinko wrote:
>
> > Well, obviously the new code is not equivalent to the old one, because
> > Long.TYPE and other TYPE constants are the classes for the primitive
> > types (long, int etc.), and those are not an instance of
> > java.lang.Number.
> >
>
>  Yep my bad. I mis-read what my IDE was telling me. That explains some of
> the original code but not all of it. Any ideas?
>
>  Mark
>
>
>  PS Better patch to follow shortly.
>
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>  For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

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


Re: svn commit: r646076 - /tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java

Posted by Mark Thomas <ma...@apache.org>.
Konstantin Kolinko wrote:
> Well, obviously the new code is not equivalent to the old one, because
> Long.TYPE and other TYPE constants are the classes for the primitive
> types (long, int etc.), and those are not an instance of
> java.lang.Number.

Yep my bad. I mis-read what my IDE was telling me. That explains some of 
the original code but not all of it. Any ideas?

Mark


PS Better patch to follow shortly.

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


Re: svn commit: r646076 - /tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
Well, obviously the new code is not equivalent to the old one, because
Long.TYPE and other TYPE constants are the classes for the primitive
types (long, int etc.), and those are not an instance of
java.lang.Number.

>From the sources:
o.a.el.lang.ELArithmetic.isNumberType()
  is used in
  o.a.el.lang.ELSupport.checkType()
    and that is used in
    o.a.jasper.compiler.Validator.checkXmlAttributes()

Looking into Validator.checkXmlAttributes(), there is
"expectedClass = JspUtil.toClass(expectedType, loader);"

and the value returned by JspUtil.toClass() may be "long.class" or
other primitive class.

It is from reading the sources. Have not run it though.



2008/4/9  <ma...@apache.org>:
>  Modified: tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java
>  URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java?rev=646076&r1=646075&r2=646076&view=diff
>  ==============================================================================
>  --- tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java (original)
>  +++ tomcat/trunk/java/org/apache/el/lang/ELArithmetic.java Tue Apr  8 14:25:04 2008
>  @@ -326,8 +326,8 @@
>          return (obj != null && isNumberType(obj.getClass()));
>      }
>
>  -    public final static boolean isNumberType(final Class type) {
>  -        return type == (java.lang.Long.class) || type == Long.TYPE || type ==  ...
>  +    public final static boolean isNumberType(final Class<?> type) {
>  +        return (Number.class.isAssignableFrom(type));
>      }
>

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