sta-szek/pojo-tester

Exception when comparing null and empty collections

Closed this issue · 0 comments

Lot of HashCodeBuilders return same hashcodes for null any empty collecions:

Class a.b.c.Message has bad 'hashCode' method implementation.
The hashCode method should return different hash codes for non equal objects.
Current implementation returns same hash codes.
Object:
a.b.c.Message@4b168fa9[id=a46f009d-bbf0-480f-bd93-2864e88f8c85,readers=[]]
and
a.b.c.Message@1a84f40f[id=a46f009d-bbf0-480f-bd93-2864e88f8c85,readers=<null>]
should have different hash codes:
-988706936
and
-988706936

problem is with implementation of increase value:

protected Deque<?> increaseValue(Deque<?> value, Class<?> type) {
        return value != null
                     ? null 
                     : new LinkedList();
    }

Try to return something else than empty collection objects.

Classes to reproduce bug:

package a.b.c;

import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.apache.commons.lang3.builder.ToStringBuilder;

import java.util.LinkedHashSet;
import java.util.Set;
import java.util.UUID;

public class Message {

    private final UUID id;
    private final Set<Person> readers;

    public Message() {
        this.id = UUID.randomUUID();
        this.readers = new LinkedHashSet<>();
    }

    @Override
    public boolean equals(final Object obj) {
        if (this == obj) {
            return true;
        }

        if (obj == null || getClass() != obj.getClass()) {
            return false;
        }

        final Message that = (Message) obj;

        return new EqualsBuilder().append(id, that.id)
                                  .append(readers, that.readers)
                                  .isEquals();
    }

    @Override
    public int hashCode() {
        return new HashCodeBuilder().append(id)
                                    .append(readers)
                                    .toHashCode();
    }

    @Override
    public String toString() {
        return new ToStringBuilder(this).append("id", id)
                                        .append("readers", readers)
                                        .toString();
    }
}
package a.b.c;

import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.apache.commons.lang3.builder.ToStringBuilder;

public class Person {

    private final String id;
    private final PersonType type;

    public Person(final String id, final PersonType type) {
        this.id = id;
        this.type = type;
    }

    @Override
    public boolean equals(final Object obj) {
        if (this == obj) {
            return true;
        }

        if (obj == null || getClass() != obj.getClass()) {
            return false;
        }

        final Person that = (Person) obj;

        return new EqualsBuilder().append(id, that.id)
                                  .append(type, that.type)
                                  .isEquals();
    }

    @Override
    public int hashCode() {
        return new HashCodeBuilder().append(id)
                                    .append(type)
                                    .toHashCode();
    }

    @Override
    public String toString() {
        return new ToStringBuilder(this).append("id", id)
                                        .append("type", type)
                                        .toString();
    }
}
package a.b.c;

public enum PersonType {
    U, A, C, S
}
package a.b.c;

import static pl.pojo.tester.api.assertion.Assertions.assertPojoMethodsFor;
import pl.pojo.tester.api.assertion.Method;

import org.junit.Test;

public class MessageTest {

    @Test
    public void shouldPojoBeWellImplemented() {
        assertPojoMethodsFor(Message.class).testing(Method.TO_STRING,
                                                    Method.EQUALS,
                                                    Method.CONSTRUCTOR,
                                                    Method.HASH_CODE)
                                           .areWellImplemented();
    }
}