Add withDefault and withDefaultValue to Map types #275
Conversation
szeiger
force-pushed
the
wip/map-with-default
branch
from
41f553d to
3ef4478
Compare
There was a problem hiding this comment.
I think we should merge this PR (after my comments are addressed) as it provides the same feature as the one that is in the current collections.
However I’m wondering if we could take advantage of the collections redesign to improve this operation: have something that returns a CC[K, V1] instead of a Map and, most importantly, keep the default value even when transformer methods are called. I think this is doable, see how I’ve done here: https://github.com/scala/collection-strawman/pull/262/files#diff-d767a48bf7524bed5c62886aa0a3e082R53
| with MapOps[K, V, Map, Map[K, V]] { | ||
|
|
||
| /** The same map with a given default function. | ||
| * Note: `get`, `contains`, `iterator`, `keys`, etc are not affected by `withDefault`. |
There was a problem hiding this comment.
Instead of giving an incomplete list of methods followed by “etc” I would clearly say what’s the impact: it provides an implementation for the default method, which is (internally) used only by the apply method.
| def result(): Map[K, V] = new WithDefault[K, V](ub.result(), d) | ||
| def add(elem: (K, V)): this.type = { ub.add(elem); this } | ||
| } | ||
| } |
There was a problem hiding this comment.
It seems that it should be possible to use mapFactory.newBuilder[K, V]().mapResult(WithDefault[K, V](_, d)) instead.
| override def updated[V1 >: V](key: K, value: V1): WithDefault[K, V1] = new WithDefault[K, V1](underlying.updated[V1](key, value), d) | ||
| override def remove (key: K): WithDefault[K, V] = new WithDefault(underlying - key, d) | ||
| override def withDefault[V1 >: V](d: K => V1): immutable.Map[K, V1] = new WithDefault[K, V1](underlying, d) | ||
| override def withDefaultValue[V1 >: V](d: V1): immutable.Map[K, V1] = new WithDefault[K, V1](underlying, x => d) |
There was a problem hiding this comment.
Why do we need to override withDefault and withDefaultValue? The implementation is the same as the base implementation.
There was a problem hiding this comment.
No, it's not. It avoids layering several WithDefault on top of each other.
|
Could you also update the README to indicate that the |
|
You can't and shouldn't return a |
There is no shared abstraction for mutable and immutable Map types because of differences in variance. This is the same as in the old collections library.
szeiger
force-pushed
the
wip/map-with-default
branch
from
3ef4478 to
65bdac8
Compare
|
Updated and rebased. |
|
I wonder if it should really be defined on PartialFunction... |
|
What behavior would you recommend? |
3 participants
There is no shared abstraction for mutable and immutable Map types
because of differences in variance. This is the same as in the old
collections library.