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

Provide default implementations of factory methods when possible #335

Closed
julienrf opened this issue Jan 9, 2018 · 2 comments
Closed

Provide default implementations of factory methods when possible #335

julienrf opened this issue Jan 9, 2018 · 2 comments
Assignees
Milestone

Comments

@julienrf
Copy link
Contributor

julienrf commented Jan 9, 2018

The fromSpecificIterable, newSpecificBuilder, and other factory methods that are currently implemented in concrete collections only could probably have default implementations earlier in the hierarchy.

@julienrf
Copy link
Contributor Author

In #379, @lrytz wrote the following:

In 2.12:

scala>   class K extends Iterable[Int] {
     |     var x = 10
     |     def iterator: Iterator[Int] = new Iterator[Int] {
     |       def hasNext = x > 0
     |       def next() = { x -= 1; x}
     |     }
     |   }
defined class K

scala>

scala> (new K).toList
res0: List[Int] = List(9, 8, 7, 6, 5, 4, 3, 2, 1, 0)

In strawman I need to add

    def iterableFactory: collection.IterableFactoryLike[Iterable] = Iterable
    protected[this] def fromSpecificIterable(coll: scala.Iterable[Int]): Iterable[Int] = iterableFactory.from(coll)
    protected[this] def newSpecificBuilder(): collection.mutable.Builder[Int, Iterable[Int]] = iterableFactory.newBuilder()

It's pretty mechanical (and also not obvious to newcomers) in the basic case when you don't need specific return types for operations. Should we provide some helper classes with default implementations?

@szeiger
Copy link
Member

szeiger commented Feb 16, 2018

The approach I had in mind runs into a bug (or a more fundamental language problem?) in Scala (but would work in Dotty): scala/bug#10725

szeiger added a commit that referenced this issue Feb 20, 2018
- The respective `CC` types are exposed (to `protected[this]`) through
  aliases `IterableCC`, `SortedIterableCC`, `MapCC` and `SortedMapCC`
  in the respective `Ops` traits, plus a `BitSetC` for the shared
  `BitSetOps` trait.

- This allows defining `fromSpecificIterable`, `newSpecificBuilder`
  and `empty` (for all but `Iterable`) in the respective collection
  traits `Iterable`, `SortedSet`, `Map` and `SortedMap` (*not* in their
  `Ops` traits) where the `CC` type is visible and set to a concrete
  type but can still be refined in subtypes. This gives us a valid
  implementation until a concrete collection type with a refined `CC`
  gets defined. In this case the inherited implementations have the
  wrong type, so the user is forced to override them.

- Implementations of `fromSpecificIterable`, `newSpecificBuilder` and
  `empty` can be removed from almost all collection implementations
  except the ones where they need to be refined even further than what
  a factory can provide (e.g. `IntMap`, `PriorityQueue`).

- Factories are generally overridden in abstract collection types that
  refine a `CC` (e.g. `Iterable` -> `Seq` -> `immutable.Set` ->
  `immutable.IndexedSeq`) so that concrete implementations without
  further refinement do not need to override them.

- DefaultMap is deprecated because there is no more boilerplate left
  that it could implement.

- `sortedFromIterable`, `mapFromIterable` and `sortedMapFromIterable`
  are treated the same way as `fromIterable`. They are implemented as
  `inline final` calls to the respective factory method.

Fixes #335
szeiger added a commit that referenced this issue Feb 20, 2018
- The respective `CC` types are exposed (to `protected[this]`) through
  aliases `IterableCC`, `SortedIterableCC`, `MapCC` and `SortedMapCC`
  in the respective `Ops` traits, plus a `BitSetC` for the shared
  `BitSetOps` trait.

- This allows defining `fromSpecificIterable`, `newSpecificBuilder`
  and `empty` (for all but `Iterable`) in the respective collection
  traits `Iterable`, `SortedSet`, `Map` and `SortedMap` (*not* in their
  `Ops` traits) where the `CC` type is visible and set to a concrete
  type but can still be refined in subtypes. This gives us a valid
  implementation until a concrete collection type with a refined `CC`
  gets defined. In this case the inherited implementations have the
  wrong type, so the user is forced to override them.

- Implementations of `fromSpecificIterable`, `newSpecificBuilder` and
  `empty` can be removed from almost all collection implementations
  except the ones where they need to be refined even further than what
  a factory can provide (e.g. `IntMap`, `PriorityQueue`).

- Factories are generally overridden in abstract collection types that
  refine a `CC` (e.g. `Iterable` -> `Seq` -> `immutable.Set` ->
  `immutable.IndexedSeq`) so that concrete implementations without
  further refinement do not need to override them.

- DefaultMap is deprecated because there is no more boilerplate left
  that it could implement.

- `sortedFromIterable`, `mapFromIterable` and `sortedMapFromIterable`
  are treated the same way as `fromIterable`. They are implemented as
  `inline final` calls to the respective factory method.

Fixes #335
szeiger added a commit that referenced this issue Feb 20, 2018
- The respective `CC` types are exposed (to `protected[this]`) through
  aliases `IterableCC`, `SortedIterableCC`, `MapCC` and `SortedMapCC`
  in the respective `Ops` traits, plus a `BitSetC` for the shared
  `BitSetOps` trait.

- This allows defining `fromSpecificIterable`, `newSpecificBuilder`
  and `empty` (for all but `Iterable`) in the respective collection
  traits `Iterable`, `SortedSet`, `Map` and `SortedMap` (*not* in their
  `Ops` traits) where the `CC` type is visible and set to a concrete
  type but can still be refined in subtypes. This gives us a valid
  implementation until a concrete collection type with a refined `CC`
  gets defined. In this case the inherited implementations have the
  wrong type, so the user is forced to override them.

- Implementations of `fromSpecificIterable`, `newSpecificBuilder` and
  `empty` can be removed from almost all collection implementations
  except the ones where they need to be refined even further than what
  a factory can provide (e.g. `IntMap`, `PriorityQueue`).

- Factories are generally overridden in abstract collection types that
  refine a `CC` (e.g. `Iterable` -> `Seq` -> `immutable.Set` ->
  `immutable.IndexedSeq`) so that concrete implementations without
  further refinement do not need to override them.

- DefaultMap is deprecated because there is no more boilerplate left
  that it could implement.

- `sortedFromIterable`, `mapFromIterable` and `sortedMapFromIterable`
  are treated the same way as `fromIterable`. They are implemented as
  `inline final` calls to the respective factory method.

Fixes #335
szeiger added a commit that referenced this issue Feb 20, 2018
- The respective `CC` types are exposed (to `protected[this]`) through
  aliases `IterableCC`, `SortedIterableCC`, `MapCC` and `SortedMapCC`
  in the respective `Ops` traits, plus a `BitSetC` for the shared
  `BitSetOps` trait.

- This allows defining `fromSpecificIterable`, `newSpecificBuilder`
  and `empty` (for all but `Iterable`) in the respective collection
  traits `Iterable`, `SortedSet`, `Map` and `SortedMap` (*not* in their
  `Ops` traits) where the `CC` type is visible and set to a concrete
  type but can still be refined in subtypes. This gives us a valid
  implementation until a concrete collection type with a refined `CC`
  gets defined. In this case the inherited implementations have the
  wrong type, so the user is forced to override them.

- Implementations of `fromSpecificIterable`, `newSpecificBuilder` and
  `empty` can be removed from almost all collection implementations
  except the ones where they need to be refined even further than what
  a factory can provide (e.g. `IntMap`, `PriorityQueue`).

- Factories are generally overridden in abstract collection types that
  refine a `CC` (e.g. `Iterable` -> `Seq` -> `immutable.Set` ->
  `immutable.IndexedSeq`) so that concrete implementations without
  further refinement do not need to override them.

- DefaultMap is deprecated because there is no more boilerplate left
  that it could implement.

- `sortedFromIterable`, `mapFromIterable` and `sortedMapFromIterable`
  are treated the same way as `fromIterable`. They are implemented as
  `inline final` calls to the respective factory method.

Fixes #335
szeiger added a commit that referenced this issue Feb 20, 2018
- The respective `CC` types are exposed (to `protected[this]`) through
  aliases `IterableCC`, `SortedIterableCC`, `MapCC` and `SortedMapCC`
  in the respective `Ops` traits, plus a `BitSetC` for the shared
  `BitSetOps` trait.

- This allows defining `fromSpecificIterable`, `newSpecificBuilder`
  and `empty` (for all but `Iterable`) in the respective collection
  traits `Iterable`, `SortedSet`, `Map` and `SortedMap` (*not* in their
  `Ops` traits) where the `CC` type is visible and set to a concrete
  type but can still be refined in subtypes. This gives us a valid
  implementation until a concrete collection type with a refined `CC`
  gets defined. In this case the inherited implementations have the
  wrong type, so the user is forced to override them.

- Implementations of `fromSpecificIterable`, `newSpecificBuilder` and
  `empty` can be removed from almost all collection implementations
  except the ones where they need to be refined even further than what
  a factory can provide (e.g. `IntMap`, `PriorityQueue`).

- Factories are generally overridden in abstract collection types that
  refine a `CC` (e.g. `Iterable` -> `Seq` -> `immutable.Set` ->
  `immutable.IndexedSeq`) so that concrete implementations without
  further refinement do not need to override them.

- DefaultMap is deprecated because there is no more boilerplate left
  that it could implement.

- `sortedFromIterable`, `mapFromIterable` and `sortedMapFromIterable`
  are treated the same way as `fromIterable`. They are implemented as
  `inline final` calls to the respective factory method.

Fixes #335
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants