New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception when comparing null and empty collections #153

Closed
sta-szek opened this Issue Dec 20, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@sta-szek
Owner

sta-szek commented Dec 20, 2016

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();
    }
}

@sta-szek sta-szek self-assigned this Dec 21, 2016

sta-szek pushed a commit that referenced this issue Dec 21, 2016

Piotr Joński

@sta-szek sta-szek added the bug label Dec 21, 2016

sta-szek pushed a commit that referenced this issue Dec 21, 2016

Piotr Joński

sta-szek pushed a commit that referenced this issue Dec 21, 2016

Piotr Joński
Merge pull request #155 from sta-szek/#153-Exception-when-comparing-n…
…ull-and-empty-collections

#153. Collection value changers create collection with one element in…

@sta-szek sta-szek closed this Dec 21, 2016

@sta-szek sta-szek removed the in progress label Dec 21, 2016

@sta-szek sta-szek added this to the 0.7.2 milestone Dec 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment