On 16/06/2015 4:58 AM, Jochen Theodorou wrote:
Am 15.06.2015 08:03, schrieb Peter Ledbrook:
> I have a different take on this. I think intersect() is by
definition a
> set operation and the method on Iterable is simply a convenience that
> automatically converts the object and the arguments to sets. I would
> even argue for the result of the method always being a set as well.
If you argue like that I have to ask: Does it make sense to convert an
Iterable to Set automatically? Wouldn't it be better to instead have it
on Set only?
Probably. But a convenience method is normally there to remove the need
for boilerplate yourself, such as the conversions to sets.
but it must make sense. And your statement questions the sense in that.
Interestingly, Paul mentions on the corresponding issue that intersect()
was originally a convenience for
a.findAll { b.contains(it) }
then it should have had another name maybe
I really wonder if we should keep the method on anything that is no
Set... actually even with sets... If a non-standard comparator is used,
problems like above will easily happen again.
equals() itself should be commutative, so only a broken implementation
would cause issues. That would then be the fault of the comparator
implementor.
equals already easily fails if you bring in subclasses. But I did mean two
sorted sets with differing comparators... They can be correct in each set, but
what would the comparator of an intersection of those be? I guess for SortedSet
with two participating comparators the result would have to be always empty.
I'd be happy to see the method deprecated and renamed if it's not
intended to be a set intersection. That would reduce or eliminate the
confusion without breaking any backwards compatibility.
I don't necessarily think the existing name is necessarily that bad.
It follows the general definition of the word intersection as per
wikipedia - just not the more specialized set intersection defn.
Having said that, I'd have no problem with something that improves
things without creating further confusion. Are we looking for names
for the existing method or for a set-based one? findAll works for
me as a replacement/parallel name - we just don't want to create
confusion with the existing mutating retainAll. Also, something
like strictIntersect might work for a Set based variant.
There are many examples of Groovy methods which aren't commutative.
The fact we have left.operation(right) has an implication that
left and right might play different roles. We try to reduce the times
when lack of commutativity causes confusion and that most frequently
means methods which map to operators and names like intersect
would be a close second as to when confusion can arise but it doesn't
mean we have to always have commutativity.
Cheers, Paul.
any naming suggestions? I wonder if simply using "findAll" would be good.
bye blackdrag
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
