1da96b7fc560c3e0c65ff4ac0872a5eb

sample java program

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
import java.util.Hashtable;

class Employee 
{
	int id;
	String name;

	Employee() {
	}

	Employee(int id, String name) {
		this.id = id;
		this.name = name;
	}

	public int hashCode() {
		return 100;
	}

	public boolean equals(Object e) {
		if (this.hashCode() == e.hashCode())
		{
			return true;
		}

		/*if (!(e instanceof Employee))
		{
			return false;
		}

		Employee emp = (Employee)e;
		
		if (this.id == emp.id && this.name == emp.name)
		{
			return true;
		}*/
		return false;
	}

	public String toString() {
		return "id = " + this.id + "; name = " + this.name;
	}
}

public class TestHash
{
	public static void main(String args[]) {

		Hashtable h = new Hashtable();
		Employee e1 = new Employee(1, "aa");
		Employee e2 = new Employee(2, "bb");

		h.put(e1, e1);
		h.put(e2, e2);

		System.out.println("\nequals ==> " + e1.equals(e2));
		
		System.out.println(h.get(e1));
		System.out.println(h.get(e2));
	}
}

Refactorings

No refactoring yet !

1da96b7fc560c3e0c65ff4ac0872a5eb

nikhil

September 28, 2007, September 28, 2007 01:47, permalink

No rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
import java.util.Hashtable;

class Employee 
{
	int id;
	String name;

	Employee() {
	}

	Employee(int id, String name) {
		this.id = id;
		this.name = name;
	}

	public int hashCode() {
		return 100;
	}

	public boolean equals(Object e) {
		if (this.hashCode() == e.hashCode())
		{
			return true;
		}

		/*if (!(e instanceof Employee))
		{
			return false;
		}

		Employee emp = (Employee)e;
		
		if (this.id == emp.id && this.name == emp.name)
		{
			return true;
		}*/
		return false;
	}

	public String toString() {
		return "id = " + this.id + "; name = " + this.name;
	}
}

public class TestHash
{
	public static void main(String args[]) {

		Hashtable h = new Hashtable();
		Employee e1 = new Employee(1, "aa");
		Employee e2 = new Employee(2, "bb");

		h.put(e1, e1);
		h.put(e2, e2);

		System.out.println("\nequals ==> " + e1.equals(e2));
		
		System.out.println(h.get(e1));
		System.out.println(h.get(e2));
	}
}
Avatar

me

September 28, 2007, September 28, 2007 02:15, permalink

No rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
class Employee 
{
	int id;
	String name;

	Employee() {
	}

	Employee(int id, String name) {
		this.id = id;
		this.name = name;
	}

	public int hashCode() {
		return 100;
	}

	public boolean equals(Object e) {
		return this.hashCode() == e.hashCode();
	}

	public String toString() {
		return "id = " + this.id + "; name = " + this.name;
	}
}
22100344f91ad34ad9fb8fb6b6940a84

Vamsee

September 28, 2007, September 28, 2007 03:31, permalink

1 rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
class Employee 
{
	int id;
	String name;
        int hashCode = 0;
	Employee() {
                this(0, "");
	}

	Employee(int id, String name) {
		this.id = id;
		this.name = name;
                if (this.name == null) this.name = "";
	}

	public int hashCode() {
              if (hashCode == 0) hashCode = (id+name).hashCode();
              return hashCode;
	}

	public boolean equals(Object e) {
		return this.hashCode() == e.hashCode();
	}

	public String toString() {
		return "id = " + this.id + "; name = " + this.name;
	}
}
Da52378cd5b4688568e9ddf13d7618cb

Rhett Maxwell

September 28, 2007, September 28, 2007 04:17, permalink

No rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
class Employee 
{
	int id;
	String name="";
        int hashCode = 0;

	Employee(int id, String name) {
		this.id = id;
		this.name = name == null ? "" : name;
                hashCode = (id+name).hashCode();
	}

	public int hashCode() {
              return hashCode;
	}

	public boolean equals(Object e) {
		return e!=null && this.hashCode() == e.hashCode();
	}

	public String toString() {
		return "id = " +id + "; name = " + name;
	}
}
Bfd5e53fb306c1c02aa3100ad91115df

Simon Hutchinson

September 28, 2007, September 28, 2007 04:48, permalink

2 ratings. Login to rate!

Rhett's suggestion is Ok but the equals method should check for the class of the object being compared. Also extending this class and providing a mutator to e.g name would result in a bug (Ok whilst immutable) Eclipse does a pretty good job with equals and hashcode (a la Joshua Bloch)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
class Employee 
{
	int id;
	String name="";
        int hashCode = 0;

	Employee(int id, String name) {
		this.id = id;
		this.name = name == null ? "" : name;                
	}

	public int hashCode() {
		final int PRIME = 31;
		int result = 1;
		result = PRIME * result + id;
		result = PRIME * result + ((name == null) ? 0 : name.hashCode());
		return result;
	}
	public boolean equals(Object obj) {
		if (this == obj)
			return true;
		if (obj == null)
			return false;
		if (getClass() != obj.getClass())
			return false;
		final Employee other = (Employee) obj;
		if (id != other.id)
			return false;
		if (name == null) {
			if (other.name != null)
				return false;
		} else if (!name.equals(other.name))
			return false;
		return true;
	}

	public String toString() {
		return new StringBuilder ("id = ").append(id).append(" name = ").append(name).toString();
	}
}
Avatar

WimW

September 28, 2007, September 28, 2007 05:15, permalink

No rating. Login to rate!

1) make class fields private.
2) removed unused hashCode class field
3) Use Commons lang package for equals, hashcode and toString
4) Validate that name is not null. An exception is thrown if it is. What is the point of making an immutable object with a null name anyway.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
import org.apache.commons.lang.Validate;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

class Employee {

    private int id;
    private String name;

    Employee(int id, String name) {

        Validate.notNull(name);

        this.id = id;
        this.name = name;
    }

    public int hashCode() {
        return new HashCodeBuilder().append(id).append(name).toHashCode();
    }

    public boolean equals(Object obj) {
        if (obj instanceof Employee == false) {
            return false;
        }
        if (this == obj) {
            return true;
        }
        Employee rhs = (Employee) obj;
        return new EqualsBuilder().appendSuper(super.equals(obj)).append(name,
                rhs.name).append(id, rhs.id).isEquals();

    }

    public String toString() {
        return ToStringBuilder.reflectionToString(this);
    }
}
Avatar

WimW

September 28, 2007, September 28, 2007 05:16, permalink

No rating. Login to rate!

1) make class fields private.
2) removed unused hashCode class field
3) Use Commons lang package for equals, hashcode and toString
4) Validate that name is not null. An exception is thrown if it is. What is the point of making an immutable object with a null name anyway.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
import org.apache.commons.lang.Validate;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

class Employee {

    private int id;
    private String name;

    Employee(int id, String name) {

        Validate.notNull(name);

        this.id = id;
        this.name = name;
    }

    public int hashCode() {
        return new HashCodeBuilder().append(id).append(name).toHashCode();
    }

    public boolean equals(Object obj) {
        if (obj instanceof Employee == false) {
            return false;
        }
        if (this == obj) {
            return true;
        }
        Employee rhs = (Employee) obj;
        return new EqualsBuilder().appendSuper(super.equals(obj)).append(name,
                rhs.name).append(id, rhs.id).isEquals();

    }

    public String toString() {
        return ToStringBuilder.reflectionToString(this);
    }
}
579f8c01fcd7ccbca3230b28d8820a84

btruelove

September 29, 2007, September 29, 2007 06:33, permalink

No rating. Login to rate!

-Made id field an Integer so it wont default to a 0 value (useful if you're using an ORM tool to save objects to a database)
-Added getters and setters for id and name to make the Employee more of a POJO (plain old java object) style that will help it to play nice with Java frameworks (e.g. Spring, Hibernate etc)
-removed appendSuper() because we don't want to inherit the referential equality comparison that the Object equals method uses. Now two identical Employee objects that have different references can be equal, for example the following is now true; new Employee(101, "Barry").equals(new Employee(101, "Barry"))

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

class Employee {

	private Integer id;

	private String name;

	public Employee() {

	}

	public Employee(Integer id, String name) {
		this.id = id;
		this.name = name;
	}

	public String getName() {
		return name;
	}

	public void setName(String name) {
		this.name = name;
	}

	public Integer getId() {
		return id;
	}

	public void setId(Integer id) {
		this.id = id;
	}

	public int hashCode() {
		return new HashCodeBuilder().append(id).append(name).toHashCode();
	}

	public boolean equals(Object obj) {
		if (obj instanceof Employee == false) {
			return false;
		}
		if (this == obj) {
			return true;
		}
		Employee rhs = (Employee) obj;
		return new EqualsBuilder().append(name,
				rhs.name).append(id, rhs.id).isEquals();
	}

	public String toString() {
		return ToStringBuilder.reflectionToString(this);
	}
}
1f393f50dc29119d656c91d6701ddbba

jeremy.weiskotten.myopenid.com/

September 29, 2007, September 29, 2007 09:49, permalink

No rating. Login to rate!

equals should NOT test for equal hashCodes. Two objects can be non-equal but have the same hashCode -- you don't want the equals method to return true for two objects that are not equal but happen to have the same hashCode.

B8d457d2c39911ea4c74ba7d66b9c3f7

Marco Valtas

October 1, 2007, October 01, 2007 11:19, permalink

No rating. Login to rate!

Some observations, for anyone who is trying to implement equals() should read: http://www.geocities.com/technofundo/tech/java/equalhash.html

Simon Hutchinson (refactoring above) provides the right way to implement a equals(). I disagree with people that inserted jakarta commons in the code because this has consequences in whole software, adding dependences increase complexity, load, size and in this special case is unnecessary. In fact in a refactoring process will be more practical only rewrite the code.

Avatar

Rob

October 2, 2007, October 02, 2007 13:19, permalink

No rating. Login to rate!

Why not use the keyword final if this is supposed to be immutable?!

Your refactoring





Format Copy from initial code

or Cancel