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

Override `stepper` in more collection types #11483

Closed
lrytz opened this Issue Apr 12, 2019 · 3 comments

Comments

Projects
None yet
3 participants
@lrytz
Copy link
Member

commented Apr 12, 2019

(source: scala/scala-java8-compat#113)

Check if there are more collections that would from a stepper override (with an existing or new stepper implementation). From the top of my head:

  • mutable/immutable ArraySeq, ArrayBuffer
  • mutable LinkedHashMap/LinkedHashSet (cannot use XTableStepper because that doesn't yield the elements in insertion order)
  • ArrayDeque
  • VectorMap
@lrytz

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

To allow doing that in a minor release, it might make sense to add some overrides of stepper in 2.13.0 without actually providing a more efficient Stepper (yet). @Ichoran, what do you think?

@Ichoran

This comment has been minimized.

Copy link

commented Apr 12, 2019

@lrytz - LinkedHashX are linked lists when following insertion order. There's no point overriding them.

VectorMap can be overridden. The existing Vector stepper should work fine mapped, but we don't have a mapped stepper any longer.

ArraySeq and ArrayBuffer can just use the stepper for arrays.

ArrayDeque has a (slightly) more complicated indexing scheme and will need a new (slightly modified) stepper.

So I'm not sure the overrides are enough given that we need new stepper classes also. I guess we could use anonymous subclasses whose type isn't observable.

@lrytz

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

We can declare new steppers private[collection], as the types don't leak out to the stepper signature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.