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

Make it possible to iterate over ListBuilder #212

Open
dlux-google opened this issue Jul 6, 2020 · 8 comments
Open

Make it possible to iterate over ListBuilder #212

dlux-google opened this issue Jul 6, 2020 · 8 comments
Assignees

Comments

@dlux-google
Copy link

dlux-google commented Jul 6, 2020

Currently the only way of iterating over a ListBuilder is build().asList(). It would be great to have a more friendly way to do it, something like a .asIterable() method.

@dlux-google
Copy link
Author

As an extra, it would be great if ListBuilder and BuiltList had a common superclass which would implement .asIterable(). Maybe add it to BuiltIterable?

@davidmorgan
Copy link
Contributor

asIterable sounds plausible.

ListBuilder can't extend BuiltIterable, because BuiltIterable implements Iterable, and Iterable methods clash with ListBuilder methods.

So I guess it would be a new interface holding asIterable. It would have a much weaker contract than BuiltIterable, because it is allowed on mutable instances. Not sure of a good name, maybe:

/// A collection that can expose its contents as an `Iterable`. The only safe way to
/// use it is to immediately copy `asIterable`.
abstract class AsIterable<T> {
  Iterable<T> asIterable();
}

@dlux-google
Copy link
Author

dlux-google commented Jul 7, 2020

Given that BuiltList will extend/implement AsIterable(), too, I would write something like that:

/// A collection that expose its contents as an [Iterable]. If the collection is modifiable,
/// then make sure the collection is not modified while using the content as [Iterable],
/// otherwise copying the content is advisable.

@samlythemanly
Copy link

Is there a particular reason ListBuilder implements Iterable-like methods that behave in intrinsically different ways than those on Iterable itself? It feels like an antipattern to mimic an Iterable while quietly mutating the base object (possibly bug-inducing), especially when it's preventing it from actually implementing Iterable and providing the expected functionality there.

@dave26199
Copy link

Yes; builders are all about mutating one instance, mixing methods that create new instances doesn't work.

If you look at the examples of using the built collection "rebuild" method, you'll see what I mean.

var list2 = list.rebuild((b) => b
    ..where((x) => x.isOdd)
    ..add(1));

If where returned a new instance then its result would be discarded rather than used.

We could instead make add return a new instance, but that would be a completely different API :) not builders at all.

@samlythemanly
Copy link

Ah, fascinating. This seems to stem from the fact that the updates function returns void rather than the builder that's being mutated. Has there been any consideration to make those functions return the builder/iterable that's being worked on (other than that being a breaking change, obviously)? replace could then be called on the result in the update function, since replace already takes in an Iterable.

For example:

// built_list.dart

BuiltList<E> rebuild(Iterable<E> Function(ListBuilder<E>) updates) =>
    (toBuilder()..update(updates)).build();

// list_builder.dart

void update(Iterable<E> Function(ListBuilder<E>) updates) =>
    replace(updates(this));

@dave26199
Copy link

If a method mutates state, it's confusing to also return it; that's how other languages do the builder pattern, the "fluent" API style, but Dart can do that for free with the cascade operator.

The fundamental thing here is that BuiltList has only non-mutating methods that return new results, and ListBuilder only mutating APIs that return nothing. As part of that it has methods that resemble Iterable methods, but it can't actually be the same.

Use of the Iterable-like methods on builders is relatively rare, it's tempting to say they should just have been omitted. But there is an important way of using built collections where you create a builder at the start of a block of code then build it at the end. This needs the more detailed methods so you don't have to keep switching between built lists and builders to get the operation you want.

@samlythemanly
Copy link

Oh, I was suggesting that just within the context of the updates function parameter of rebuild, update, etc; but not anything else!

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

No branches or pull requests

4 participants