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

cannot extend built-in list implementation #2600

Closed
jmesserly opened this issue Apr 16, 2012 · 9 comments
Closed

cannot extend built-in list implementation #2600

jmesserly opened this issue Apr 16, 2012 · 9 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug

Comments

@jmesserly
Copy link

There are various places where we'd like to implement List<E>. It'd be nice to be able to extend the default growable implementation, for cases like an ObservableList where we just want to do some change tracking. This works great in other languages (e.g. Java and C#), but in Dart it has a few issues:
   * the class has different names in the VM vs compiled to JS (See http://dartbug.com/949)
   * Even if that was fixed GrowableObjectArray can't be subclassed in the VM.

Otherwise, what we end up doing is forwarding all of the methods:

class ObservableList<E> implements List<E> {
  <...>

  Iterator<E> iterator() => _items.iterator();

  void forEach(void f(E element)) => _items.forEach(f);

  Collection map(f(E element)) => _items.map(f);

  <...>

  void operator []=(int index, E value) {
    _items[index] = value;
    notifyChanged(...);
  }

  void set length(int newLength) {
    _items.length = newLength;
    notifyChanged(...);
  }

  void add(E value) {
    _items.add(value);
    notifyChanged(...);
  }

  <...>

This has unnecessary overhead--memory for the wrapper and extra generated JS code.

@DartBot
Copy link

DartBot commented Jan 21, 2013

This comment was originally written by @sethladd


Brought up on Stack Overflow: http://stackoverflow.com/questions/14441620/custom-collection-in-dart

@jmesserly
Copy link
Author

It's interesting that the workaround discussed in that question uses noSuchMethod, which has bad code size implications on JS, and probably performance overhead too (on VM or JS).


Added Library-Core label.

@kevmoo
Copy link
Member

kevmoo commented Apr 3, 2013

An option: extend Iterable<E> and implement List<E>

You only have to implement Iterator&lt;E> get iterator and you get all of the impl in Iterable for free.

@jmesserly
Copy link
Author

@kevmoo Yup. IIRC that's what we're currently doing in web_ui (see lib/observe/) Still has a lot of extra methods copied though, and the overhead issues mentioned above.

@jmesserly
Copy link
Author

@DartBot
Copy link

DartBot commented Apr 8, 2013

This comment was originally written by @simonpai


@kevmoo, I think extending from Collection will save a few more methods for us.

I strongly agree with John. It's kind of silly, in my opinion, to hide such a common implementation so that every project which needs to hack just a few things on the List needs to reinvent the wheel in its code base.

This feels like you are doing some woodwork and realize there is a chainsaw right behind a thin door, but you have to use your handsaw instead just because the door is locked for some unexplained reason.

@lrhn
Copy link
Member

lrhn commented Apr 8, 2013

I hope we can have a ListBase class you can extend to get full List functionality by implementing just [], []=, length and length=.
It would be great if that could be List itself, but since the "List" constructor is already used for other purposes, and List has no generative constructor at all, that would be inconvenient.


Added Accepted label.

@jmesserly
Copy link
Author

that would be lovely!

@floitschG
Copy link
Contributor

ListBase and ListMixin are now available in dart:collection.


Added Fixed label.

@jmesserly jmesserly added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core labels May 2, 2013
@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. library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants