https://bz.apache.org/bugzilla/show_bug.cgi?id=62287
--- Comment #4 from Mark Thomas <ma...@apache.org> --- (In reply to Christopher Schultz from comment #3) > 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. My assumption was that hashCode() was used originally because it was faster. Therefore the patch uses hashCode() and only falls back to equals if the hash codes are the same. Whether that makes sense depends on the relative speed of hashCode() vs equals() and how often the tested objects are equal or not. I opted not to dig in to that and go with the patch as written. I've no objection to dropping the hashCode() if that is what folks prefer. > 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"? Not that I can think of. There would only ever be a single EL implementation in use. -- 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
