Member Avatar for chixm8_49

Hello again. :)
I am comparing two nodes (some of my other methods need this method). I am having a problem with this method because it returns false even though both nodes have the same content. What are the problems of this code? Any suggestions? This is the critical part of my code.

By the way, here's the code:

public boolean equals(Object o){		
		if(o instanceof Node){ //checks whether the object is an instance of Node
			System.out.println("Entering the first if statement"); // meaning o is an instance of Node
			if(((Node)o).elname == this.elname && ((Node)o).efname == this.efname && ((Node)o).work == this.work && ((Node)o).floorno == this.floorno){ //checking the contents of Nodes
				System.out.println("Entering the 2nd if and returned true"); //Contents are the same
				return true;
			}else{
				System.out.println("Entering the second if and returned false"); //contents are different
				return false;
			}
		}else{
			System.out.println("Entering the first if and returned false"); //O is not an instance of Node
			return false;
		}
	}
  • 2 Contributors
  • 2 Replies
  • 2K Views
  • 6 Hours Discussion Span
  • Latest Post Latest Post by chixm8_49

Dani AI

Generated

nailed the root cause: == on Strings checks whether two variables point to the exact same object, not whether the text is the same. That is why two nodes with identical content can still compare false. Also watch for null fields; calling .equals on a null reference will throw an NPE. A few best practices will make this rock-solid and future-proof.

If you are on Java 7+, prefer a null-safe comparison using Objects.equals for reference fields and == for primitives. Also, whenever you override equals, you must override hashCode so your nodes behave correctly in HashSet/HashMap.

import java.util.Objects;

@Override
public boolean equals(Object o) {
    if (this == o) return true;               // quick check: same object
    if (o == null || getClass() != o.getClass()) return false; // or use instanceof if subclasses should be equal
    Node other = (Node) o;
    return this.floorno == other.floorno
        && Objects.equals(this.elname, other.elname)
        && Objects.equals(this.efname, other.efname)
        && Objects.equals(this.work, other.work);
}

@Override
public int hashCode() {
    return Objects.hash(elname, efname, work, floorno);
}

Notes and gotchas:

  • If you intend subclasses of Node to compare equal, switch the getClass() check to instanceof and handle symmetry carefully.
  • If case should be ignored for names, use equalsIgnoreCase (but document that choice).
  • Keep fields used in equality stable; changing them after insertion into a set or map will break lookups.
  • Add @Override to catch mistakes at compile time and write a couple of unit tests that cover equal, not equal, and null cases.

This keeps the spirit of ’s code but avoids reference comparisons and null pitfalls while honoring the equals/hashCode contract.

Recommended Answers

All 2 Replies

Member Avatar for masijade

If some of those "things" are Strings, use String's equals method rather than == to compare them. (I'm assuming efname is a String.)

Edited by masijade because: n/a
Member Avatar for chixm8_49

If some of those "things" are Strings, use String's equals method rather than == to compare them. (I'm assuming efname is a String.)

*cries for joy*
It works! Thank you very much! :)

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.