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

[Edson Tirelli]

   I see what you are saying and I think it works. Although, it will be
heavier, it would cover scenarios not covered by the current implementation.

   My suggestion is if you can, please create a test case for the problem,
change the collectSet accumulate function to do what you propose and send a
pull request. I will review it and apply. I think in this case, making
user's life easier by not requiring immutability might trump performance,
but we need to evaluate some tests just in case.

   Edson

On Tue, Feb 28, 2012 at 1:01 PM, SirMungus <Patrick_Rusk at ssga.com> wrote:

>
> Edson Tirelli-4 wrote
> >
> >    As you noticed already, the function uses the resulting object as a
> key
> > in the map because the goal is indeed to match equals() objects and
> filter
> > them to return a set, no duplicates. Of course, because in case of (b)
> the
> > hashcode changes, java no longer finds the key to remove it from the map.
> >
> >    The problem of using a straight identity map is that it will no longer
> > collect sets of equals() object, but sets of identical objects, what is
> > obviously not the intent of the function. For instance, in case (a), if
> > two
> > objects have the same first and last name, there will be 2 instances of
> > the
> > resulting object created, even if they are equals(). That will break the
> > function.
> >
> Thanks for the attention you've been able to give this, Edson.
> The solution I've proposed doesn't break the equals() contact of the
> resulting set; it just applies it at getResult() time, rather than during
> accumulate()/reverse(). So, the main question would be whether the creation
> of a new HashSet in getResult() is too costly, which I've discussed
> elsewhere on this thread.
> Are there existing stress tests that could test that empirically?
>
> --
> View this message in context:
> http://drools.46999.n3.nabble.com/BUG-5-3-0-Final-CollectSetAccumulateFunction-should-probably-use-IdentityHashMap-internally-tp3774079p3784789.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
>



-- 
  Edson Tirelli
  JBoss Drools Core Development
  JBoss by Red Hat @ www.jboss.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/rules-dev/attachments/20120228/3cf50910/attachment-0001.html