Map.keys and values

I suggest that whenever the meaning (type, whether it has side
effects, etc.) of an identifier changes, there should be a short spell
of that identifier not existing, or at least being deprecated.

V1

trait Map[K, V] {
def keys: Iterator[K]
def values: Iterator[V]
}

V2

trait Map[K, V] {
def keysIterator: Iterator[K]
def valuesIterator: Iterator[V]
}

V3

trait Map[K, V] {
def keys: Set[K]
def values: Set[V]
}

Anyone who uses all 3 versions will be told of the change, though
possibly irritated. Anyone who goes from V1 to V3 will be frustrated
as Iulian was.

How much time to leave between each version depends on the users, of course.

2009/6/2 Iulian Dragos :
> The new collection library have silently changed the meaning of methods keys
> and values for a map: before they returned iterators, now they return a set.
> While this makes (some) sense, I think it's a very bad decision and can
> break a lot of code. I was chasing a bug for a few hours yesterday, after
> porting the specialized branch to the new collection library. I have no
> words to describe the frustration I felt when I realized what was happening.
>
> Instead, I propose to make keys and values default to keysIterator and
> valuesIterator. Anyone against this change?
>
> iulian
>
> --
> Iulian Dragos
> =============
> EPFL-IIF-LAMP 1
> INR 321 http://lamp.epfl.ch/~dragos
>
>

2009/6/2 Iulian Dragos :
> The new collection library have silently changed the meaning of methods keys
> and values for a map: before they returned iterators, now they return a set.
> While this makes (some) sense, I think it's a very bad decision and can
> break a lot of code. I was chasing a bug for a few hours yesterday, after
> porting the specialized branch to the new collection library. I have no
> words to describe the frustration I felt when I realized what was happening.
>
> Instead, I propose to make keys and values default to keysIterator and
> valuesIterator. Anyone against this change?

Yes. keys and values returning iterators is really very poor design.
At the very least if they're going to be kept this way they should go
through a deprecation / removal / recreation as the correct types
phase, which takes forever to do the right thing.

Could you elaborate on the bug this caused?

The biggest problem with the new design (which needs fixing) is that
values should *not* be a set, just an Iterable. It doesn't make sense
for it be so, and think that's the issue most likely to cause bizarre
bugs.

On Tue, 2009-06-02 at 10:20 +0100, David MacIver wrote:
> Yes. keys and values returning iterators is really very poor design.
> At the very least if they're going to be kept this way they should go
> through a deprecation / removal / recreation as the correct types
> phase, which takes forever to do the right thing.
>
> Could you elaborate on the bug this caused?
>
> The biggest problem with the new design (which needs fixing) is that
> values should *not* be a set, just an Iterable. It doesn't make sense
> for it be so, and think that's the issue most likely to cause bizarre
> bugs.

Agreed on both counts. The fact that keys and values returned iterators
in previous versions is something that annoyed me several times.

Best,
Ismael

David MacIver wrote:
> 2009/6/2 Iulian Dragos :
>
>> The new collection library have silently changed the meaning of methods keys
>> and values for a map: before they returned iterators, now they return a set.
>> While this makes (some) sense, I think it's a very bad decision and can
>> break a lot of code. I was chasing a bug for a few hours yesterday, after
>> porting the specialized branch to the new collection library. I have no
>> words to describe the frustration I felt when I realized what was happening.
>>
>> Instead, I propose to make keys and values default to keysIterator and
>> valuesIterator. Anyone against this change?
>>
>
> Yes. keys and values returning iterators is really very poor design.
>
Can you elaborate on this? I would prefer to have an Iterable instead of
an Iterator, but my biggest concern was the silent change. On the one
hand, we go to great lengths to keep some sort of source compatibility,
on the other hand we change the meaning of existing code. By great
lengths I mean the new language feature called package objects, whose
selling point was the fact that you can type alias scala.List to
scala.collection.immutable.List.

> At the very least if they're going to be kept this way they should go
> through a deprecation / removal / recreation as the correct types
> phase, which takes forever to do the right thing.
>
Absolutely, deprecation is the only way we have to signal a method needs
attention, so there you go.
> Could you elaborate on the bug this caused?
>
> The biggest problem with the new design (which needs fixing) is that
> values should *not* be a set, just an Iterable. It doesn't make sense
> for it be so, and think that's the issue most likely to cause bizarre
> bugs.
>
That's exactly it. I was relying on keys and values having the same
length. On some tests it worked, because values happened to be distinct.
In others, it crashed. Since my code was working on some generic
interface that both Sets and Iterables implemented, this compiled fine.

iulian

I think you have a point, Iulian. Can you work with Gilles to
deprecate keys and values, and have them return iterators again? We
can have keySet/keyIterator as the new members.

Cheers

On Tue, Jun 2, 2009 at 11:34 AM, martin odersky wrote:
> I think you have a point, Iulian. Can you work with Gilles to
> deprecate keys and values, and have them return iterators again? We
> can have keySet/keyIterator as the new members.

Isn't the issue more to do with the unexpected set-like instead of
bag-like semantics for values rather than being an issue with the
switch from Iterator?

Cheers,

Miles

2009/6/2 Miles Sabin :
> On Tue, Jun 2, 2009 at 11:34 AM, martin odersky wrote:
>> I think you have a point, Iulian. Can you work with Gilles to
>> deprecate keys and values, and have them return iterators again? We
>> can have keySet/keyIterator as the new members.
>
> Isn't the issue more to do with the unexpected set-like instead of
> bag-like semantics for values rather than being an issue with the
> switch from Iterator?

Exactly. Making values a Set breaks the semantics quite badly. However
keys are already implicitly a set (you can't have duplicate keys) and
making values an Iterable with allowed repetitions is not problematic.

Making keys a Set and values an Iterable achieves source level
compatibility in 90% of cases because of the common methods on
Iterator and Iterable, and the remaining 10% of cases it gives a
compile error (because you'll end up doing things like calling next
and hasNext on an Iterable), so is a relatively harmless change
compared to the benefits it brings.

Hello.

Iulian and I just discussed the problem with "Map.values", and we
think that David's proposition (quoted below) is good. I'll change
"Map.values" to return an Iterable.

Cheers,
Gilles.

>>> I think you have a point, Iulian. Can you work with Gilles to
>>> deprecate keys and values, and have them return iterators again? We
>>> can have keySet/keyIterator as the new members.
>>
>> Isn't the issue more to do with the unexpected set-like instead of
>> bag-like semantics for values rather than being an issue with the
>> switch from Iterator?
>
> Exactly. Making values a Set breaks the semantics quite badly. However
> keys are already implicitly a set (you can't have duplicate keys) and
> making values an Iterable with allowed repetitions is not problematic.
>
> Making keys a Set and values an Iterable achieves source level
> compatibility in 90% of cases because of the common methods on
> Iterator and Iterable, and the remaining 10% of cases it gives a
> compile error (because you'll end up doing things like calling next
> and hasNext on an Iterable), so is a relatively harmless change
> compared to the benefits it brings.

On Tue, Jun 2, 2009 at 3:31 PM, Gilles Dubochet wrote:
> Hello.
>
> Iulian and I just discussed the problem with "Map.values", and we think that
> David's proposition (quoted below) is good. I'll change "Map.values" to
> return an Iterable.

OK, fine with me. Ignore what I wrote previously, that was lacking context.

Cheers