ibatis-user-java mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Butler <jeffgbut...@gmail.com>
Subject Re: iBator EqualsHashCodePlugin: NullPointerExceptions in equal method
Date Wed, 25 Mar 2009 15:39:40 GMT
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 <benjamin@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
>
>
>
>

Mime
View raw message