“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(a)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-CollectSetAccumulateFun...
Sent from the Drools: Developer (committer) mailing list mailing list
archive at
Nabble.com.
_______________________________________________
rules-dev mailing list
rules-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/rules-dev