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

List.map should specialise the return type of Collection.map #4054

Closed
DartBot opened this issue Jul 11, 2012 · 6 comments
Closed

List.map should specialise the return type of Collection.map #4054

DartBot opened this issue Jul 11, 2012 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jul 11, 2012

This issue was originally filed by sammcca...@google.com


Collection.map() returns a Collection.
In practice I think List.map() always returns a List, but it would be nice if we could rely on this!

@lrhn
Copy link
Member

lrhn commented Jul 11, 2012

I agree that if we have a map function on List, it should be declared to return a List too. That's possible, but it's not optimal.
I'd actually prefer to have a constructor instead, something like
  class List<T> ...
    List.map(Iterable source, T mapping(var value)) {...}
so you can decide for each call which result type you want.
That would allow you to write both:
  new List<X>.map(myList, (v) => v.p);
and:
  new Set<X>.map(myList, (v) => v.p);
which will create different kinds of collections and have the correct generic types as well.

Perhaps we should also have the utility 'map' instance method that works on the current collection and creates another of the same kind, but it's not obvious how to get a proper generic type for the result collection. That really requires generic methods (or reflection, but I'd prefer to not depend on that in what should be simple collection operations).


Removed Type-Defect label.
Added Type-Enhancement, Area-Library, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jul 11, 2012

This comment was originally written by @seaneagan


This exact same issue exists for Collection#filter(issue #1062), so both should be solved consistently. I think using constructors instead of instance methods for Collection transformations breaks down when considering chaining. Instance methods are more fluent and readable. More importantly though, if using constructors, I assume they will eagerly copy each element of the input. When chaining this causes intermediate Collections which copy each element to be created, which will not even be used, and the result cannot act as a "view" which updates when the original Collection changes. This is discussed in more detail in issue #1305.

I'm not sure why generic methods are seen as more complicated than generic constructors, they seem like the obvious solution to Collection#map and Collection#reduce (issue #1649). The "as" cast operator might also help.

@lrhn
Copy link
Member

lrhn commented Jul 12, 2012

Using constructors isn't as fluent as methods.
The filter (aka findAll) method is simple, because it can return the same type of collection as the original (same generic type).
For a map method, it'll probably end up untyped, at least until we get generic methods (if that ever happens).
I think we should definitely have the constructors, for uses where the generic type actually matters, but likely also the instance methods for more casual use.

If we have both, then we have more choices wrt. making the generated values lazy. E.g., we could have an Iterator<T>.map(Iterable source, T mapping(var value)) constructor that creates a lazy iterator, while Set<T>.map(Iterable source, T mapping(var value)) eagerly creates a populated set object. The instance method can then be eager (which is what I would expect for an "operator-like" method), since you have a lazy alternative anyway.

You should also be able to get special views of existing collections, e.g., a Set view of the keys of a Map, or a Collection view of its values. These would be actively backed by the original collection and reflect changes.
I.e., I see three ways to create collections from other collections:

  • Use an appropriate constructor for the resulting collection type. This may be lazy.
  • Use a (limited capability) instance method on the original collection.
  • Create a view of an existing collection.
    Then Iterator, and poossibly Collection, constructors will be lazy, since they are abstract collections with no storage strategy, while Map, Set and List (and any future concrete collections with random access) will have constructors that always create a new instance eagerly.

@DartBot
Copy link
Author

DartBot commented Jul 12, 2012

This comment was originally written by @seaneagan


I would expect the fluent methods (including "map" and "filter"/"findAll") to also return "views", so that you can build up composite transformations without ever having to do any copying or unnecessary iteration. You can then compose these with the existing copy constructors in the less common case of needing a "snapshot". Also, if generic methods might ever make it into Dart, why not just rely on the "as" cast operator as a temporary work around, instead of adding duplicative constructors? Example:

var strings = <String>['bar', '5', 'bAz', 'BaZ'];

// usually just need a view
// assume isAlpha and toUpperCase are Functions
var upperAlphas = strings.findAll(isAlpha).map(toUpperCase) as List<String>;

// can later add an item to "strings" and have it reflected in "view"
strings.add("foo");

// strings never needs to be fully iterated or copied to do the following
assert(upperAlphas.contains("FOO"));

// can snapshot a view using a copy constructor
var snapshot = new List<String>.from(upperAlphas);

// can even use a copy constructor from a different type
var snapshot = new Set<String>.from(upperAlphas);

@lrhn
Copy link
Member

lrhn commented Mar 24, 2015

Added NotPlanned label.

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue labels Mar 24, 2015
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants
@kevmoo @lrhn @DartBot and others