Avatar

Hi all,

Ive been doing some practice classes recently and am having trouble implementing the equals method on this class. [line 47]

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
public class Bus {
    private String TYPE;
    private int MAX_RANGE;
    private int maxPassengers;
    private int noPassengers;

    public Bus(String TYPE, int maxRange, int maxPassengers) {
       maxRange = 0;
       maxPassengers = 30;
       TYPE = "School";
    }

    public String getType() {
       return TYPE;
    }

    public int getMaxRange() {
       return MAX_RANGE;
    }

    public int getMaxPassengers() {
       return maxPassengers;
    }

    public void setMaxPassengers(int pass) {
       maxPassengers = 30;
    }

    public int load(int numPassengers) {
      if (numPassengers > maxPassengers) {
          noPassengers = maxPassengers;
          System.out.println(maxPassengers - numPassengers);
      }
      else if (noPassengers == numPassengers) {
      }
      return -1;
    }

    public void unload() {
        noPassengers = 0;
    }

    public double calculateRange() {
       return MAX_RANGE - (MAX_RANGE * 0.5 * noPassengers / maxPassengers);
    }

    //equals method to compare TYPE and MAX_RANGE
    public boolean equals(Object o) {
    

    }

    public String toString() {
       return "The wheels on the bus go round and round";
    }

}

Refactorings

No refactoring yet !

4d72203c38dd5f3e3d2d446b5888e8a7

Elij

November 5, 2007, November 05, 2007 12:49, permalink

No rating. Login to rate!

Once you've unboxed o you should be able to access it's private fields as you're within the class.

Then you can just compare the value types and strings.

995e3bfeeaae71d6edcf0ecb96bdf786

silly.bear

November 5, 2007, November 05, 2007 13:14, permalink

No rating. Login to rate!

if it's ony about comparing TYPE and MAX_RANGE:

1
2
3
4
5
6
7
8
public boolean equals(Object o) 
{
    Bus tmpObj = (Bus) o;

    return (tmpObj.getType().equals(TYPE)) && 
           (tmpObj.getMaxRange()== MAX_RANGE);

 }
Avatar

MadiC

November 5, 2007, November 05, 2007 13:51, permalink

No rating. Login to rate!

Isn't perfectly done. Maybe lack of an assert rule or default return (false), in example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
    //equals method to compare TYPE and MAX_RANGE
    public boolean equals(Object o) {
    
        Bus tmpObj = (Bus) o;

        if (!(o instanceof Bus))
            return false;


        return (tmpObj.getType().equals(TYPE)) && 
               (tmpObj.getMaxRange()== MAX_RANGE);
    }
995e3bfeeaae71d6edcf0ecb96bdf786

silly.bear

November 5, 2007, November 05, 2007 13:53, permalink

No rating. Login to rate!

that's better :D

B8d457d2c39911ea4c74ba7d66b9c3f7

Marco Valtas

November 5, 2007, November 05, 2007 16:09, permalink

No rating. Login to rate!

Wheres the hashCode() implementation?

Here is a complete description of what you need to know about equals() and hashCode().

http://www.geocities.com/technofundo/tech/java/equalhash.html

and here is a old refactoring post about the subject of equals() and hashCode()

http://refactormycode.com/codes/27-sample-java-program#refactor_160

Hope this help.

0a6a9ee3c06dc328ed58ff76a6fce166

Hadrien

November 5, 2007, November 05, 2007 17:12, permalink

No rating. Login to rate!

using exceptions.

But an equals method that does not care about all private fields is not that consistent to me.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
    //equals method to compare TYPE and MAX_RANGE
    public boolean equals(Object o)
    {
        try{    
        
            Bus tmpObj = Bus.class.cast(o);

            return (tmpObj.getType().equals(TYPE)) && 
                   (tmpObj.getMaxRange()== MAX_RANGE);
        
        }catch(ClassCastException e){
            return false;
        }
    }
2a96614a070c79c3bdf73c89a112182a

Hadrien again

November 5, 2007, November 05, 2007 17:15, permalink

No rating. Login to rate!

Marco Valtas is right : according to java reference:
Two equals objects must have the same hashCode (for use in an HashSet/hashtable for example)

995e3bfeeaae71d6edcf0ecb96bdf786

silly.bear

November 5, 2007, November 05, 2007 18:21, permalink

No rating. Login to rate!

i know i'm not perfect, but you teached me something new!!

really tnx!

7e9f93b6e882404b2d044829eaec6076

Peter Hawkins

November 5, 2007, November 05, 2007 22:47, permalink

No rating. Login to rate!

no try/catch needed here. test the object's class before casting, i.e., you know it's a Bus when you cast it.

1
2
3
4
5
6
7
8
9
10
11
12
13
//equals method to compare TYPE and MAX_RANGE
public boolean equals(Object o) 
{
     if (!(o instanceof Bus))
          return false;

     Bus tmpObj = (Bus) o;

     return (tmpObj.getType().equals(TYPE)) && 
          (tmpObj.getMaxRange() == MAX_RANGE);
}
Avatar

Tim Büthe

November 6, 2007, November 06, 2007 21:13, permalink

1 rating. Login to rate!

You guys definitly should check the Apache Commons Lang project: http://commons.apache.org/lang/ there are EqualsBuilder and HashCodeBuilder in it: http://commons.apache.org/lang/userguide.html#lang_builder__

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

November 7, 2007, November 07, 2007 19:49, permalink

No rating. Login to rate!

Your constructor is all wrong... Try the following.

Also, you might want to consider making type, maxRange, and maxPassengers final and immutable (remove the setter methods, only initialize them through the constructor). Otherwise, a Bus that is used in a Set or as a key on a Map can have a mutable identity, and you won't be able to find it.

1
2
3
4
5
6
7
8
9
10
11
public class Bus {
    private String type;
    private int maxRange;
    private int maxPassengers;
    private int noPassengers;

    public Bus(final String type, final int maxRange, final int maxPassengers) {
       this.maxRange = maxRange;
       this.maxPassengers = maxPassengers;
       this.type = type;
    }
Avatar

olivier.fayau.myopenid.com

November 13, 2007, November 13, 2007 00:15, permalink

No rating. Login to rate!

You should explain what you wanted to do.
I think it's something like this...

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
62
63
64
65
66
67
public class Bus {

	private String type = "School";
	private int maxRange = 0;
	private int maxPassengers = 30;
	private int noPassengers = 0;

	public Bus() {
	}
	
	public Bus(final String type, final int maxRange, final int maxPassengers) {
		this.type = type;
		this.maxRange = maxRange;
		this.maxPassengers = maxPassengers;
	}

	public String getType() {
		return this.type;
	}

	public int getMaxRange() {
		return this.maxRange;
	}

	public int getMaxPassengers() {
		return this.maxPassengers;
	}

	public void setMaxPassengers(int pass) {
		this.maxPassengers = pass;
	}

	public int load(int numPassengers) {
		if (this.noPassengers + numPassengers <= maxPassengers) {
			this.noPassengers += numPassengers;
			System.out.println("Rest: " + (this.maxPassengers - this.noPassengers));
		}
		return this.noPassengers;
	}

	public void unload() {
		this.noPassengers = 0;
	}

	public double calculateRange() {
		return this.maxRange - (this.maxRange * 0.5 * this.noPassengers / this.maxPassengers);
	}

	public String toString() {
		return "The wheels on the bus go round and round";
	}

	// equals method to compare type and maxRange
	public boolean equals(Object o) {
		if (o == null || !(o instanceof Bus))
			return false;
		Bus tmpObj = (Bus) o;
		if (this.maxRange != tmpObj.getMaxRange())
			return false;
		if (this.type == null && tmpObj.getType() != null)
			return false;
		if (!this.type.equals(tmpObj.getType()))
			return false;
		return true;
	}

}
5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

November 16, 2007, November 16, 2007 19:58, permalink

No rating. Login to rate!

null is not an instanceof Bus.

1
2
3
4
// if (o == null || !(o instanceof Bus))

if (!(o instanceof Bus))
   return false;
2155c92be66863c4634778bf522efe14

armandino.myopenid.com

December 8, 2007, December 08, 2007 07:13, permalink

No rating. Login to rate!

I usually implement equals methods along these lines...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
public boolean equals(Object obj)
{
    boolean isEqual = false;

    if(obj != null && obj instanceof Bus)
    {
        Bus bus = (Bus)obj;

        isEqual = bus.getType().equals(getType())
               && bus.getMaxRange() == getMaxRange());
    }

    return isEqual;
}
Bd667dc3fd7c9af2123e77bcec5d133b

techguide

March 8, 2008, March 08, 2008 20:20, permalink

No rating. Login to rate!

Thanks guys, exactly what I was looking for...

Avatar

asdasd

May 7, 2008, May 07, 2008 15:08, permalink

No rating. Login to rate!

this is safe, but cleaner, and maybe even faster imho. remember to override hashcode.

1
2
3
4
5
6
7
8
9
public boolean equals(Object obj)
{
    return obj instanceof Bus && equals((Bus)obj);
}
public boolean equals(Bus obj)
{
    return bus.getType().equals(getType())
               && bus.getMaxRange() == getMaxRange());
}
Avatar

sdjk

May 10, 2008, May 10, 2008 10:20, permalink

No rating. Login to rate!

The above code only works when you don't use inheritance. Hence I wouldn't call it safe.
Oh and for the same reason it's a good habit to use .getClass over instanceof in equals methods.
Your toString needs work as well imo :)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
	public boolean equals(Object obj)
	{
		if (this == obj)
			return true;
		if (obj == null)
			return false;
		if (getClass() != obj.getClass())
			return false;
		Bus other = (Bus) obj;
		if (maxRange != other.maxRange)
			return false;
		if (type == null)
		{
			if (other.type != null)
				return false;
		}
		else if (!type.equals(other.type))
			return false;
		return true;
	}

Your refactoring





Format Copy from initial code

or Cancel