Skip to content
This repository was archived by the owner on Dec 22, 2021. It is now read-only.

Introduce IterableOnceOps as common abstraction for Iterable & Iterator #480

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

szeiger
Copy link
Contributor

@szeiger szeiger commented Feb 22, 2018

Fixes #465

@szeiger
Copy link
Contributor Author

szeiger commented Feb 23, 2018

Looks like an actual unsoundness in SortedMultiSet slipped through the cracks of the compiler(s) so far but gets flagged now that zipWithIndex was moved to a supertrait.

- Unlike the other Ops types, IterableOnceOps is *not* implemented by
  IterableOnce but only by Iterator and Iterable.

- Fix the hierarchy of MultiSets and SortedMultiSets: The refactoring
  caused soundness problems to show up that were not reported before.
  These types now follow the usual diamond pattern where
  mutable/immutable extends generic and sorted extends unsorted.
@szeiger szeiger force-pushed the wip/iterableonceops branch from 60c1065 to 4b21b76 Compare February 24, 2018 14:20
@szeiger
Copy link
Contributor Author

szeiger commented Feb 24, 2018

Rebased on top of #457

with MultiSetOps[A, MultiSet, MultiSet[A]] {

override def iterableFactory: IterableFactory[MultiSet] = MultiSet
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on the problem that is solved by introducing the separation MultiSet/MultiSetImpl?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh maybe that’s for making SortedMultiSet a subtype of MultiSet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you want the usual diamond pattern with all of:

  • mutable.SortedMultiSet <: collection.SortedMultiSet

  • immutable.SortedMultiSet <: collection.SortedMultiSet

  • mutable.MultiSet <: collection.MultiSet

  • immutable.MultiSet <: collection.MultiSet

  • collection.SortedMultiSet <: collection.MultiSet

  • mutable.SortedMultiSet <: mutable.MultiSet

  • immutable.SortedMultiSet <: immutable.MultiSet

This requires that MultiSet is a trait.

/** This implementation trait can be mixed into an `IterableOnce` to get the basic methods that are shared between
* `Iterator` and `Iterable`. The `IterableOnce` must support multiple calls to `iterator()` but may or may not
* return the same `Iterator` every time.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should define the documentation variables that are not inherited but used in the operations’ scaladoc.

@szeiger szeiger merged commit a12d083 into master Feb 27, 2018
@szeiger szeiger deleted the wip/iterableonceops branch February 27, 2018 14:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants