Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing collections methods in Forwarders, Proxies, SynchronizedXxx #4290

Closed
scabug opened this issue Feb 23, 2011 · 9 comments
Closed

missing collections methods in Forwarders, Proxies, SynchronizedXxx #4290

scabug opened this issue Feb 23, 2011 · 9 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Feb 23, 2011

I'm opening a separate ticket from #4288 so I can close that when I fix slice.

Here are the methods I found in either Traversable or TraversableOnce, but not in TraversableForwarder, excluding the methods singled out for exclusion in TraversableForwarder's scaladoc. The list is so long I'm afraid I'm missing something about how this is supposed to work.

I realize that it's not uncommon that one can get away with not forwarding some methods because they are implemented in terms of other methods (and given that fundamental methods like map and flatMap are on this list, I assume that logic was being applied) but with nothing in place to verify that that is the case for any given combination of methods, nothing checking if it is made untrue via method overrides, nothing documenting the intentions, and nothing preventing some future change from making it untrue, it seems like we have to forward everything if we want to even have a glimpse of correctness.

Here is the opening list. I am sure there are more once we move upward from Traversable.

collect
collectFirst
drop
dropWhile
filter
filterNot
flatMap
groupBy
init
inits
isTraversableAgain
map
maxBy
minBy
partition
scanLeft
scanRight
slice
span
splitAt
tail
tails
take
takeWhile
toParIterable
toParMap
toParSeq
toParSet
toTraversable
withFilter
@scabug
Copy link
Author

scabug commented Feb 23, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4290?orig=1
Reporter: @paulp
See #4288

@scabug
Copy link
Author

scabug commented Feb 23, 2011

@paulp said:
I can feel my brain sprouting cracks so I'll just leave this for now. Given some high level documentation of how the pieces are supposed to fit together, and a believable strategy for keeping all the classes in sync, I imagine I can implement it. Until then there are too many question marks for me to want to dive in. I wouldn't even have stuck a toe in were it not for ListBuffer using a *Forwarder in a fundamental way.

@scabug
Copy link
Author

scabug commented Feb 23, 2011

@paulp said:
I add the 2+2=4 observation that in the absence of other bugs and inconsistencies, given that the Forwarder implements Traversable, all the methods must get where they're going one way or another. Problems like this one emerge when there are outright inconsistencies in how methods are implemented (like slice in Traversable vs. slice everywhere else.) And there is no indication why some methods are forwarded and not others. Technically all you have to forward is foreach, right? So beyond that one method, what is driving the selection?

@scabug
Copy link
Author

scabug commented Feb 24, 2011

@paulp said:
In the end, I'll say the real story is that there are 25 concrete implementations of slice in the collections. TWENTY-FIVE. Suddenly it's less than surprising that there are at least three distinct behaviors around the corners.

We are not exactly hitting it out of the park if we can't manage with fewer than 25 variations of slice.

@scabug
Copy link
Author

scabug commented Dec 12, 2012

@retronym said:
See also SynchronizedQueue from #4916

@scabug
Copy link
Author

scabug commented Oct 20, 2013

@retronym said:
Mirco outlines the serious issues with SynchronizedXxx in the (closed as duplicate) #7925:

Synchronized collections are broken in their design and, even worst, they currently don't fullfil their contract, i.e., most of the methods aren't synchronized!
Here is a comment in SynchronizedMap that should definitely make you nervous:
todo: also add all other methods.
Similar comments can be found in the other Synchronized collection traits.
Furthermore, these traits are broken in design as ticket https://issues.scala-lang.org/browse/SI-6087 demonstrates.
For the above reasons, I'd highly recommend to 1) add the missing synchronized methods, and 2) deprecate all Synchronized collection traits before shipping 2.11.
However, I realize that providing synchronized collections may still be desiderable (not everyone may be happy to migrate toward a concurrent collection - though, I'd like to know what's the argument for that). For this reason, I think we could provide Synchronized wrappers like Java does. This won't help people that were mixing a Synchronized trait with other collection traits, but I really don't see how we could allow that, and still prevent users from shooting themselves in the foot. If you have a great idea, please do share it.
This ticket is based on the following discussion https://groups.google.com/forum/#!topic/scala-internals/1gCbqTRCxAQ

@scabug
Copy link
Author

scabug commented Nov 25, 2013

@retronym said:
Forwarders and Proxies have been deprecated: scala/scala#3103

That change doesn't offer a plan for ListBuffer, though.

@scabug
Copy link
Author

scabug commented Dec 2, 2013

@Ichoran said:
Closing as wontfix since we've deprecated instead. Opening a new ticket to migrate ListBuffer.

@scabug
Copy link
Author

scabug commented Dec 2, 2013

@Ichoran said:
Maintenance burden is too great to maintain all of these manually. Deprecated instead of fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants