vlsi commented on PR #693:
URL: https://github.com/apache/jmeter/pull/693#issuecomment-1301792127
@undera do you have any opinion on
org.apache.jmeter.testelement.AbstractTestElement#equals ?
Currently `AbstractTestElement#hashCode` uses `identityHashCode`, so it was
supposed to use reference equality when being a part of hashMap.
However, `AbstractTestElement#equals` used `propMap.equal(other.propMap)`,
so if someone accidentally puts `TestElement` in a `HashMap`, then they could
get wrong results if hashCode values accidentally collide.
In this PR, I tried replacing the corresponding `HashMap` usages in JMeter
with `IdentityHashMap`. It turns out it required many changes. The "good" part
is that it keeps backward compatibility.
However, all the users would have to figure out and perform the same
replacements.
There's another possibility: change `AbstractTestElement#equals` to `final
equals(Object o) { return this == o; }`.
Then it would automatically support `AbstractTestElement` in `HashMap` and
`ConcurrentHashMap`.
At the same time we could add `AbstractTestElement#contentEquals` for those
who need to compare contents of the `AbstractTestElement` instead of their
identity.
It looks like making `AbstractTestElement#equals` to compare object identity
(this==that) would break backward compatibility (e.g. if someone used .equals
to compare test element contents), however, it would automatically support
`TestElements` in `Set<...>`, `Map<...`.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
