ibatis-user-java mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Klatt" <benja...@bar54.de>
Subject AW: iBator EqualsHashCodePlugin: NullPointerExceptions in equal method
Date Wed, 25 Mar 2009 11:46:33 GMT
Hi all,

sorry, i missed to post another modification on the plugin.
But may be this is a question on how you expect your equals() method to
work.

We assume that a test like this:

    Product p3 = new Product();
    Product p4 = new Product();
    assertTrue("Empty products should be equal",p3.equals(p4));

Should run fine. 
The current implementation does return false instead of true for this case.
The reason for this is that in every line like this:
this.getId() == null ? other == null : this.getId().equals(other.getId())

The other object is checked to be null. 
First, this is not necessary, while this case is handled before in the
equals method. 
Second, to make this work with the test above, this comparision should be
about the current attribute of the other object not the object itself.

To make this work the plugin code has to be modified like this:

sb.append("this."); //$NON-NLS-1$
sb.append(getterMethod);

// the following three lines are the modified ones
sb.append("() == null ? other."); //$NON-NLS-1$
sb.append(getterMethod);
sb.append("() == null : this."); //$NON-NLS-1$

sb.append(getterMethod);
sb.append("().equals(other."); //$NON-NLS-1$
sb.append(getterMethod);
sb.append("())"); //$NON-NLS-1$


Cheers
Benjamin

-----Ursprüngliche Nachricht-----
Von: Benjamin Klatt [mailto:benjamin@bar54.de] 
Gesendet: Mittwoch, 25. März 2009 13:32
An: user-java@ibatis.apache.org
Betreff: iBator EqualsHashCodePlugin: NullPointerExceptions in equal method

Hey all,

we found an issue about the equal method.
Imagine you have a data object

---------
|Product|
---------
|id     |
|name   |
|desc   |
---------

And generate the java classes as well as the equal methods (id = Integer,
name&desc = String).

The Code

    Product p1 = new Product();
    p1.setId(1);
    p1.setName("p1");
    Product p2 = new Product();
    p2.setId(1);
    assertFalse("same id but diff names should return false",p1.equals(p2));

will result in a NullPointerException.
This results from the Boolean expression in the equal method. 
The combination of

return nullCheckAttribute1 ? nullCheckObject1 : equalAttribute1
	&& nullCheckAttribute2 ? nullCheckObject2 : equalAttribute2
	&& nullCheckAttribute3 ? nullCheckObject3 : equalAttribute3 ...

what happens:

nullCheckAttribute1 = false -> equalAttribute1
equalAttribute1 = true && nullCheckAttribute2 = false -> equalAttribute2
equalAttribute2 = false && nullCheckAttribute3 == false -> equalAttribute3

equalAttribute3 will result in the NullPointerException because in the
example this means
this.getDesc().equals(other.getDesc()
and this.getDesc() already returns null.

So to fix this issue parenthesis should be added in the following way:

return nullCheckAttribute1 ? nullCheckObject1 : equalAttribute1
	&& (nullCheckAttribute2 ? nullCheckObject2 : equalAttribute2
	&& (nullCheckAttribute3 ? nullCheckObject3 : equalAttribute3 ...))

The required code change in the plugin is:

        boolean first = true;
        int numberOfParenthesis = 0; //new counter for parenthesis
        Iterator<IntrospectedColumn> iter = introspectedColumns.iterator();
        while (iter.hasNext()) {
            IntrospectedColumn introspectedColumn = iter.next();
...
            if (!iter.hasNext()) {
            	for (int i = 0; i < numberOfParenthesis; i++) {
            		sb.append(')'); // append closing parenthesis
				}
                sb.append(';');
            }

I am sorry to posting this on the users mailing list instead of fixing the
code directly, but I don't have access to the repository.

@Jeff: It would be nice to see this integrated in the next version and to
throw away out adopted plugin ;)


All the best
Benjamin

 


Mime
View raw message