You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user-java@ibatis.apache.org by Benjamin Klatt <be...@bar54.de> on 2009/03/25 12:32:12 UTC
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
----------------------
Benjamin Klatt
Mob.: +49 (0)179 - 979 55 46
Tel.: +49 (0)721 - 208 84 06
Kentuckyallee. 88
76149 Karlsruhe
Germany
Bahnhofstr. 21
37445 Walkenried
Germany
Re: iBator EqualsHashCodePlugin: NullPointerExceptions in equal
method
Posted by Jeff Butler <je...@gmail.com>.
Nice catch - thanks!
I fixed the error and added some tests to verify. All is available in
SVN now and will be in the next release.
Jeff Butler
On Wed, Mar 25, 2009 at 6:46 AM, Benjamin Klatt <be...@bar54.de> wrote:
> 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
>
>
>
>
AW: iBator EqualsHashCodePlugin: NullPointerExceptions in equal method
Posted by Benjamin Klatt <be...@bar54.de>.
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