0

The aplication will say if the user has guessed the two combination of colors that he has introduced.

I'm using a HashMap for saving an "TwoColors" object and a boolean. The TwoColors class is the next:

public class TwoColors{
    public MyColor color1;
    public MyColor color2;
    public TwoColors(MyColor color1, MyColor color2){
        this.color1 = color1;
        this.color2 = color2;
    }
    @Override
    public boolean equals(Object obj) {
        TwoColors o = (TwoColors) obj;
        return color1 == o.color1 && color2 == o.color2;
    }
}

And MyColor is an enum

public enum MyColor{
     RED,BLUE,YELLOW,BROWN;
}

I test to put a TwoColor object key and print its value

public static void main(String[] args){
    HashMap<TwoColors, Boolean> hash = new HashMap<TwoColors, Boolean>();
    hash.put(new TwoColors(MyColor.RED,MyColor.BLUE),new Boolean(true));
    System.out.println(hash.get(new TwoColors(MyColor.RED,MyColor.BLUE)));
}

The above code outputs null although I have overriden TwoColors' equals method. Any idea what am I missing here?

Alejandro Garcia
  • 1,171
  • 1
  • 11
  • 22

2 Answers2

2

When you override equals you should always override hashcode, if you don't do that when you try to lookup the value by calling get, your hashmap cannot find it.

Read this post for understanding.

Default hashcode which you can get using Eclipse:

@Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((color1 == null) ? 0 : color1.hashCode());
        result = prime * result + ((color2 == null) ? 0 : color2.hashCode());
        return result;
    }

Also your equals implementation is not Production Code, meaning it doesn't implement it correctly. Your implementation is open for ClassCastException since you are casting it to MyColor blindly, so is your implementation prone to NullPointerException.

It should resemble something like this:

@Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        TwoColors other = (TwoColors) obj;
        if (color1 != other.color1)
            return false;
        if (color2 != other.color2)
            return false;
        return true;
    }

Before checking field value check if both references are same, check for null, check for class equality and finally check for field values.

Though Apache also has HashCodeBuilder and EqualsBuilder

Community
  • 1
  • 1
mprabhat
  • 20,107
  • 7
  • 46
  • 63
1

From the Java documentation:

Note that it is generally necessary to override the hashCode method whenever this method [equals] is overridden, so as to maintain the general contract for the hashCode method, which states that equal objects must have equal hash codes.

Since MyColor is an enum, adding this code to your class should fix it:

@Override
public int hashCode() {
    return color1.ordinal() + 31*color2.ordinal();
}
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523