Commons Lang
  1. Commons Lang
  2. LANG-259

ValuedEnum.compareTo(Object other) not typesafe - it easily could be...

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.2
    • Component/s: None
    • Labels:
      None
    • Environment:

      all

      Description

      int org.apache.commons.lang.enums.ValuedEnum.compareTo(Object other)
      is not typesafe - if the int-values are the same, it will return "0" even for two totally different sub-classes of ValuedEnum

        Issue Links

          Activity

          Hide
          Stephen Colebourne added a comment -

          Fix compareTo to check type
          rv 432748

          Show
          Stephen Colebourne added a comment - Fix compareTo to check type rv 432748
          Hide
          Stephen Colebourne added a comment -

          Does anyone believe that the classloader tricks in Enum are useful? They seem like a Bad Idea now, and are broken, even for equals.

          Is it too backwards incompatible to remove the classloader code?

          Show
          Stephen Colebourne added a comment - Does anyone believe that the classloader tricks in Enum are useful? They seem like a Bad Idea now, and are broken, even for equals. Is it too backwards incompatible to remove the classloader code?
          Hide
          Henri Yandell added a comment -

          In an effort to get 2.2 out sooner rather than later, I'm assiging all the Enum issues to 2.3. There's been no work on them currently and might be best to focus on them in a 2.3 release instead of trying to squeeze them into the 2.2 release.

          Hopefully this will keep the 2.3 release scope pretty tight.

          Show
          Henri Yandell added a comment - In an effort to get 2.2 out sooner rather than later, I'm assiging all the Enum issues to 2.3. There's been no work on them currently and might be best to focus on them in a 2.3 release instead of trying to squeeze them into the 2.2 release. Hopefully this will keep the 2.3 release scope pretty tight.
          Show
          Ralf Hauser added a comment - see also http://issues.apache.org/struts/browse/SB-20 SB-20 and LANG-258
          Hide
          Ralf Hauser added a comment -

          similarly, it should detect double entries of this kind...

          public static final int JAVA1_0_VALUE = 100;
          public static final int JAVA1_0_0VALUE = 100;
          ...

          public static final JavaVersionEnum JAVA1_0 = new JavaVersionEnum( "Java 1.0", JAVA1_0_VALUE );
          public static final JavaVersionEnum JAVA1_0_0 = new JavaVersionEnum( "Java 1.0.0", JAVA1_0_0VALUE );

          ------
          ps, the last line in the previous comment should have been

          public static final JavaVersionEnum JAVA1_0_0 = new JavaVersionEnum( "Java 1.0.0", JAVA1_0_VALUE );

          Show
          Ralf Hauser added a comment - similarly, it should detect double entries of this kind... public static final int JAVA1_0_VALUE = 100; public static final int JAVA1_0_0VALUE = 100; ... public static final JavaVersionEnum JAVA1_0 = new JavaVersionEnum( "Java 1.0", JAVA1_0_VALUE ); public static final JavaVersionEnum JAVA1_0_0 = new JavaVersionEnum( "Java 1.0.0", JAVA1_0_0VALUE ); ------ ps, the last line in the previous comment should have been public static final JavaVersionEnum JAVA1_0_0 = new JavaVersionEnum( "Java 1.0.0", JAVA1_0_VALUE );
          Hide
          Ralf Hauser added a comment -

          furthermore, it would be great if the classe's init would also detect if multiple int values are the same:

          e.g.

          public static final JavaVersionEnum JAVA1_0 = new JavaVersionEnum( "Java 1.0", JAVA1_0_VALUE );
          public static final JavaVersionEnum JAVA1_0_= = new JavaVersionEnum( "Java 1.0.0", JAVA1_0_VALUE );

          Show
          Ralf Hauser added a comment - furthermore, it would be great if the classe's init would also detect if multiple int values are the same: e.g. public static final JavaVersionEnum JAVA1_0 = new JavaVersionEnum( "Java 1.0", JAVA1_0_VALUE ); public static final JavaVersionEnum JAVA1_0_= = new JavaVersionEnum( "Java 1.0.0", JAVA1_0_VALUE );
          Hide
          Ralf Hauser added a comment -

          Before just comparing the
          iValue - ((ValuedEnum) other).iValue
          it should be checked that the classes are the same!

          Enum otherEnum = (Enum) other;
          String thisClassName = this.getClass().getName();
          String otherClassName = otherEnum.getClass().getName();
          if (!otherClassName.equals(thisClassName))

          { log.warn("enum classes are not equal \"" + thisClassName + "\" <> \"" + otherClassName + "\""); throw new ClassCastException("enum classes are not equal \"" + thisClassName + "\" <> \"" + otherClassName + "\""); }

          Furthermore, it might be recommendable to make some methods of org.apache.commons.lang.enums.Enum at least "protected" such that ValuedEnum can use them too:

          If available, I would love to also have the more sophisticated checks, Enum.compareTo(Object other) does also used in ValuedEnum.compareTo(Object other):

          if (res == 0) {
          String otherName = otherEnum.getName();
          if (other.getClass() != this.getClass()) {
          if (other.getClass().getName().equals(this.getClass().getName()))

          { return iName.compareTo(getNameInOtherClassLoader(other)); }

          }
          res = iName.compareTo(otherName);
          if (res != 0)

          { log.warn("enum names are not equal \"" + otherName + "\" <> \"" + this.getName() + "\""); throw new ClassCastException("enum names are not equal \"" + otherName + "\" <> \"" + this.getName() + "\""); }

          }

          Show
          Ralf Hauser added a comment - Before just comparing the iValue - ((ValuedEnum) other).iValue it should be checked that the classes are the same! Enum otherEnum = (Enum) other; String thisClassName = this.getClass().getName(); String otherClassName = otherEnum.getClass().getName(); if (!otherClassName.equals(thisClassName)) { log.warn("enum classes are not equal \"" + thisClassName + "\" <> \"" + otherClassName + "\""); throw new ClassCastException("enum classes are not equal \"" + thisClassName + "\" <> \"" + otherClassName + "\""); } Furthermore, it might be recommendable to make some methods of org.apache.commons.lang.enums.Enum at least "protected" such that ValuedEnum can use them too: If available, I would love to also have the more sophisticated checks, Enum.compareTo(Object other) does also used in ValuedEnum.compareTo(Object other): if (res == 0) { String otherName = otherEnum.getName(); if (other.getClass() != this.getClass()) { if (other.getClass().getName().equals(this.getClass().getName())) { return iName.compareTo(getNameInOtherClassLoader(other)); } } res = iName.compareTo(otherName); if (res != 0) { log.warn("enum names are not equal \"" + otherName + "\" <> \"" + this.getName() + "\""); throw new ClassCastException("enum names are not equal \"" + otherName + "\" <> \"" + this.getName() + "\""); } }

            People

            • Assignee:
              Unassigned
              Reporter:
              Ralf Hauser
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development