“When I use a word,” Humpty Dumpty said, in rather a scornful tone, “it means just what I choose it to mean—neither more nor less.”
“The question is,” said Alice, “whether you can make words mean so many different things.”
“The question is,” said Humpty Dumpty, “which is to be master that’s all.”
If a fact represents an entity with an immutable property set identifying it and a mutable set of state properties I define equals() to mean "is the same entity as".
If a fact represents a proposition I require equals() - taking into account all properties - to mean "declares the same as", and uphold the notion that such a fact shouldn't be modified at all, just retracted.
You can collect java.util.Sets of both, but don't violate the respective semantics.
And here's a quote from java.util.Set's javadoc: Note: Great care must be exercised if mutable objects are used as set elements. The behavior of a set is not specified if the value of an object is changed in a manner that affects equals comparisons while the object is an element in the set.
I'd say that this caveat extends to the usage of collectSet().
Of course, properly catching NPEs - even if they result from abuse - should be done whenever appropriate.
-W
On 27 February 2012 17:53, SirMungus
<Patrick_Rusk@ssga.com> wrote:
Mark Proctor wrote
>
> accumulate supports custom functions, just add your own version of
> collectSet that uses IdentityMap.
>
I could do that, but please let me take one more stab, just in case I
haven't been clear enough.
1) I am not suggesting that collectSet() return an identity-based set. I am
content to have it return a normal set, though I had early confusion on that
point.
2) The discussion above about stated vs. justified facts turns out to be a
distraction to the main point of the bug that I believe to be in
CollectSetAccumulateFunction. My failure to understand the distinction is
not germane to the bug.
3) I am not disagreeing with the value behind implementing equals() to only
take into account invariant fields, though I am stating that goes beyond the
explicity documentation of equals()/hashCode() and is explicitly *not*
followed by many classes in the JRE (most notably, collections).
There is nothing about the purpose of collectSet() that logically suggests
that it should only work with objects that have invariant
equals()/hashCode() functions. If five objects match a rule, they should be
able to be collected into a set (which may end up with fewer than five
objects). If things change such that only three facts match the rule, they
should still be able to be collected into a set. There's no compelling
reason why that should only apply to objects with invariant equals(). And it
seems to me that you rarely want to leave an NPE in code, rather than
catching it and throwing a more intelligible exception (e.g., "Object to be
removed from collectSet() was not found").
If the internal maintenance of state in CollectSetAccumulateFunction used
identity semantics and applied the equality semantics only when you asked it
for its results, it would maintain the same external behavior (returning a
"normal" equality-based set) without failing to work for objects with
variant equals.
If you Google around for the proper implementation of equals(), you will
find many serious sites that discuss it at length, yet provide examples
like:
public boolean equals(Object aThat){
if ( this == aThat ) return true;
if ( !(aThat instanceof Car) ) return false;
Car that = (Car)aThat;
return
EqualsUtil.areEqual(this.fName, that.fName) &&
EqualsUtil.areEqual(this.fNumDoors, that.fNumDoors) &&
EqualsUtil.areEqual(this.fGasMileage, that.fGasMileage) &&
EqualsUtil.areEqual(this.fColor, that.fColor) &&
Arrays.equals(this.fMaintenanceChecks, that.fMaintenanceChecks);
//array!
}
Ironically, that is explicitly against Wolfgang's suggestion above!
--
View this message in context: http://drools.46999.n3.nabble.com/BUG-5-3-0-Final-CollectSetAccumulateFunction-should-probably-use-IdentityHashMap-internally-tp3774079p3781313.html
Sent from the Drools: Developer (committer) mailing list mailing list archive at Nabble.com.
_______________________________________________
rules-dev mailing list
rules-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/rules-dev