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