https://bz.apache.org/bugzilla/show_bug.cgi?id=62287

--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
It's a little late, but I have a few comments about the patch.

1. There is no reason to check the hashcode if you are going to call .equals.
Presumably, Node.equals() knows the best way to compare most efficiently.

2. I don't know enough about ValueExperssionImpl (etc.) to know whether this is
true, but often using "instanceof" in an equals(Object) method is
inappropriate. Is there any reason to use a concrete class here rather than an
interface? Is there any reason two objects might have different runtime
classes, yet still be considered "equals"?

Under normal circumstances, a proper equals(Object) method would look like
this:

{
  return null != o
      && o.getClass().equals(this.getClass()
      && ((HighestPossibleInterface)o).getNode().equals(this.getNode())
  ;
}

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to