QDox
  1. QDox
  2. QDOX-232

DefaultJavaMethod#equals incorrect code

          Details

          • Type: Bug Bug
          • Status: Resolved Resolved
          • Priority: Major Major
          • Resolution: Fixed
          • Affects Version/s: 2.0-alpha
          • Fix Version/s: 1.12.1, 2.0
          • Component/s: Java API
          • Labels:
          • Number of attachments :
            0

            Description

            public boolean equals(Object obj) {
        if (obj == null) return false;
        JavaMethod m = (JavaMethod) obj;

        if (m.isConstructor() != isConstructor()) return false;

        if (m.getName() == null) return (getName() == null); // this line(215) is incorrect
        if (!m.getName().equals(getName())) return false;
        
        if (m.getReturns() == null) return (getReturns() == null); // THIS line(218) is incorrect

            They should be: 

                if(m.getName()==null && getName()!=null) return false;

            and 

                if(m.getReturns()==null && getReturns()!=null) return false;

            Otherwise, if two methods with same name and return type but different parameters will return true, which is incorrect.

            Maybe other `equals` have same issue. I think it's a good idea to provide a helper method to check, e.g.:

                public static boolean eq(Object obj1, Object obj2) {
        return obj1==obj2 || (obj1!=null && obj1.equals(obj2);
    }

    if(!eq(m.getName(), getName()) return false;
    if(!eq(m.getReturns(), getReturns()) return false;

              Activity

              Robert Scholte made changes -
              Field Original Value New Value
              Assignee Robert Scholte [ rfscholte ]
              Robert Scholte made changes -
              Description public boolean equals(Object obj) {
                      if (obj == null) return false;
                      JavaMethod m = (JavaMethod) obj;

                      if (m.isConstructor() != isConstructor()) return false;

                      if (m.getName() == null) return (getName() == null); // this line(215) is incorrect
                      if (!m.getName().equals(getName())) return false;
                      
                      if (m.getReturns() == null) return (getReturns() == null); // THIS line(218) is incorrect


              They should be:

                  if(m.getName()==null && getName()!=null) return false;

              and

                  if(m.getReturns()==null && getReturns()!=null) return false;

              Otherwise, if two methods with same name and return type but different parameters will return true, which is incorrect.

              Maybe other `equals` have same issue. I think it's a good idea to provide a helper method to check, e.g.:

                  public static boolean eq(Object obj1, Object obj2) {
                      return obj1==obj2 || (obj1!=null && obj1.equals(obj2);
                  }

                  if(!eq(m.getName(), getName()) return false;
                  if(!eq(m.getReturns(), getReturns()) return false;
              		{code}
              public boolean equals(Object obj) {
                      if (obj == null) return false;
                      JavaMethod m = (JavaMethod) obj;

                      if (m.isConstructor() != isConstructor()) return false;

                      if (m.getName() == null) return (getName() == null); // this line(215) is incorrect
                      if (!m.getName().equals(getName())) return false;
                      
                      if (m.getReturns() == null) return (getReturns() == null); // THIS line(218) is incorrect
              {code}

              They should be:
              {code}
                  if(m.getName()==null && getName()!=null) return false;
              {code}
              and
              {code}
                  if(m.getReturns()==null && getReturns()!=null) return false;
              {code}
              Otherwise, if two methods with same name and return type but different parameters will return true, which is incorrect.

              Maybe other `equals` have same issue. I think it's a good idea to provide a helper method to check, e.g.:
              {code}
                  public static boolean eq(Object obj1, Object obj2) {
                      return obj1==obj2 || (obj1!=null && obj1.equals(obj2);
                  }

                  if(!eq(m.getName(), getName()) return false;
                  if(!eq(m.getReturns(), getReturns()) return false;
              {code}
              Hide
              Permalink
              Freewind added a comment - - edited 
              if(m.getName()==null && getName()!=null) return false;
if (!m.getName().equals(getName())) return false;

              is not correct. If they both null, the 2nd line will throw NPE. So it's best to use that `eq(...)` helper method:

              if(!eq(m.getName(), getName()) return false;
              Show
              Freewind added a comment - - edited if (m.getName()== null && getName()!= null ) return false ; if (!m.getName().equals(getName())) return false ; is not correct. If they both null, the 2nd line will throw NPE. So it's best to use that `eq(...)` helper method: if (!eq(m.getName(), getName()) return false ;
              Hide
              Permalink
              Robert Scholte added a comment -

              If you take it from an Object point of view you're right: you have to keep in mind that both lhs and rhs can be null.
              But based on the language: what is a method without a name?
              Anyhow, there are still some optimilizations possible.
              I'll have a look at it.

              Show
              Robert Scholte added a comment - If you take it from an Object point of view you're right: you have to keep in mind that both lhs and rhs can be null. But based on the language: what is a method without a name? Anyhow, there are still some optimilizations possible. I'll have a look at it.
              Hide
              Permalink
              Robert Scholte added a comment -

              2.0 hasn't been released yet, so I've changed the affected version to 2.0-alpha. Will be fixed in the 2.0 release.

              Show
              Robert Scholte added a comment - 2.0 hasn't been released yet, so I've changed the affected version to 2.0-alpha. Will be fixed in the 2.0 release.
              Robert Scholte made changes -
              Fix Version/s 2.0 [ 15636 ]
              Affects Version/s 2.0-alpha [ 17493 ]
              Affects Version/s 2.0 [ 15636 ]
              Hide
              Permalink
              Robert Scholte added a comment -

              Fixed in rev. 1268

              Show
              Robert Scholte added a comment - Fixed in rev. 1268
              Robert Scholte made changes -
              Status Open [ 1 ] Resolved [ 5 ]
              Resolution Fixed [ 1 ]
              Robert Scholte made changes -
              Resolution Fixed [ 1 ]
              Status Resolved [ 5 ] Reopened [ 4 ]
              Hide
              Permalink
              Robert Scholte added a comment -

              Fixed in rev.1532 for QD0x-1.12.1 patch release

              Show
              Robert Scholte added a comment - Fixed in rev.1532 for QD0x-1.12.1 patch release
              Robert Scholte made changes -
              Status Reopened [ 4 ] Resolved [ 5 ]
              Fix Version/s 1.12.1 [ 18944 ]
              Resolution Fixed [ 1 ]

                People

                • Assignee:
                  Robert Scholte
                  Reporter:
                  Freewind
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  1 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: