[rules-dev] BUG: [5.3.0.Final] CollectSetAccumulateFunction should probably use IdentityHashMap internally

[Wolfgang Laun]

I don't think that this would be a good idea. Consider, for instance,
    IdentityHashMap<String,Integer> map =
              new IdentityHashMap<String,Integer>();
    map.put( "one", 1 );
    map.put( new String( "one" ), 2 );
which results in kay and value sets:
    keys: [one, one]
    values: [2, 1]

"equals" should not be based on mutable properties reflecting state,
just like a body paint job doesn't change the identity of your car -
it just makes the Joneses think so ;-)

-W

On 25/02/2012, SirMungus <Patrick_Rusk at ssga.com> wrote:
> [First post. I apologize in advance if I'm following the wrong protocol to
> report a bug.]
>
> I had a rule using the collectSet() accumulator that was giving me an NPE at
> line 123 of CollectSetAccumulateFunction.java, highlighted below:
>
>     public void reverse(Serializable context,
>                         Object value) throws Exception {
>         CollectListData data = (CollectListData) context;
>         CollectListData.MutableInt counter = data.map.get( value );
>         */if( (--counter.value) == 0 ) {/*
>             data.map.remove( value );
>         }
>     }
>
> When looking in the debugger, I could see that the "value" passed in was
> indeed present within the "data.map". However, the "data.map.get(value)" was
> returning null. I thankfully realized this was because the object had
> changed, which changed its hashCode(). The stored hashCode() in the HashMap
> was different from its current hashCode().
>
> I solved my problem by reimplementing my hashCode() as
> System.identityHashCode(this) and my equals as "this == other".
>
> However, I suspect the CollectSetAccumulateFunction should really use an
> IdentityHashMap in this situation. It's a really bad idea to allow mutable
> objects as keys in HashMap, and there is nothing about the collectSet()
> accumulator function that suggests that it should only collect immutable
> objects.
>
> Thanks.
>
> P.S. I'm loving Drools. Great tool.
>
> --
> View this message in context:
> http://drools.46999.n3.nabble.com/BUG-5-3-0-Final-CollectSetAccumulateFunction-should-probably-use-IdentityHashMap-internally-tp3774079p3774079.html
> Sent from the Drools: Developer (committer) mailing list mailing list
> archive at Nabble.com.
> _______________________________________________
> rules-dev mailing list
> rules-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-dev
>