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

KtMap.map(MapEntry<K, V> -> R) : KtList<R> #79

Closed
passsy opened this issue Feb 20, 2019 · 7 comments · Fixed by #85 or #86
Closed

KtMap.map(MapEntry<K, V> -> R) : KtList<R> #79

passsy opened this issue Feb 20, 2019 · 7 comments · Fixed by #85 or #86

Comments

@passsy
Copy link
Owner

passsy commented Feb 20, 2019

Kotlin allows mapping of entries in a map using .map

mapOf(1 to "foo", 2 to "bar")
        .map { "${it.key} -> ${it.value}" }
        .forEach(::print)
// 1 -> foo
// 2 -> bar

This leads to a name clash with KtMap.map which returns the Dart Map.

Currently all kt.dart collections allow access to the dart equivalents.

KtIterable.iter -> Iterable
KtList.list -> List
KtMap.map -> Map
KtSet.set -> Set

We have to find a way to provide standardized ways to access the dart equivalents. Additionally we should make it as easy as possible to make kt.dart collections useable using for loops.

// kt.dart 0.5.0
for(final i in listOf(1, 2, 3).iter) {
  print(i);
}
@passsy
Copy link
Owner Author

passsy commented Feb 20, 2019

One way of solving it would be to convert the interop properties to functions called asMap(), asList(), asSet() and asList().

But this would clash with the existing KtIterable<T>.asIterable(): KtIterable<T>

And for loops would look even worse

for(final i in listOf(1, 2, 3).asIterable()) {
  print(i);
}

@passsy
Copy link
Owner Author

passsy commented Feb 20, 2019

We could keep iter but create functions for all the others. iter is the only usage where a short name is useful. for is the only dart language construct which works directly with a collection.

KtIterable.iter -> Iterable
KtList.asList() -> List
KtMap.asMap() -> Map
KtSet.asSet() -> Set

@passsy
Copy link
Owner Author

passsy commented Feb 25, 2019

On could name the map function mapEntries and avoid naming collisions

@passsy
Copy link
Owner Author

passsy commented Mar 2, 2019

mapEntries() wouldn't be a great solution because it wouldn't be consistent with mapNotNull()

@christoph-jerolimov
Copy link

christoph-jerolimov commented Mar 8, 2019

From other languages I would expect that KtMap.map is a mapping function similar to your first Kotlin example, and exactly what the title of this issue describes. This is also the behaviour in the Dart List:

void main() {
  [1, 2, 3, 4, 5]
    .map((i) => i * 2)
    .forEach(print);
}

I would prefer another (new?) naming convention to get the standard Dart objects from the Kt* objects. I would renamed them to asList, asMap, ... or toList, toMap.

I didn't yet understand the problem with asIterable. Why is this required in this example? Shouldn't the KtList implements Iterable already?

for(final i in listOf(1, 2, 3).asIterable()) {
  print(i);
}

@passsy
Copy link
Owner Author

passsy commented Mar 8, 2019

Context why KtList does not implement Iterable and instead offers a .iter property.
https://github.com/dart-lang/language/issues/226

@christoph-jerolimov
Copy link

christoph-jerolimov commented Mar 8, 2019

Okay, thanks for the context. I understand now that the Iterable interface is much bigger than expected..

With this step back, I didn't see a big point why a map method is needed. Esp. mapping over a map is always a little bit tricky. Are the code maps over the key or values or both?

Readable code is so important, that I think its also great that KtMap has already the options to map over the keys, values and entries with KtMap.keys, KtMap.values and KtMap.entries. Or? Maybe you can push the documentation or example into this direction and show the benefits of this.

mapFrom({ "a": 1, "b": 2, "sum": 3 })
  .entries
  .filter((entry) => entry.key != "sum")
  .map((entry) => entry.value)
  .map((value) => value * 2)
  .forEach(print);

Hopefully you will get some other feedback as well. For me, I'm fine with the current version. 😏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants