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 <benja...@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:benja...@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
>
>
>
>
