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

Added replaceWhere method in IterableExtensions #241

Closed
wants to merge 1 commit into from
Closed

Added replaceWhere method in IterableExtensions #241

wants to merge 1 commit into from

Conversation

stevenosse
Copy link

Added the replaceWhere method to IterableExtensions.
This method returns a copy of the Iterable where all values matching a given predicate are replaced with the given one.

Example:

final List<int> samples  = [1, 2, 3, 4, 5];
print(samples.replaceWhere(predicate: (value) => value <= 2, value: 0)); // Output: (0, 0, 3, 4, 5)

@natebosch
Copy link
Member

Can you give an example use case? This feels quite specialized to me.

The version using a ternary conditional and .map also feels readable to me.

print(samples.replaceWhere(predicate: (value) => value <= 2, value: 0));
print(samples.map((s) => s <= 2 ? 0 : s));

@kevmoo
Copy link
Member

kevmoo commented May 31, 2022

What @natebosch – map is the easy solution to this now!

@stevenosse
Copy link
Author

Can you give an example use case? This feels quite specialized to me.

The version using a ternary conditional and .map also feels readable to me.

print(samples.replaceWhere(predicate: (value) => value <= 2, value: 0));
print(samples.map((s) => s <= 2 ? 0 : s));

I understand it well.
"replaceWhere" is just syntactic sugar. The method would be useful (in my opinion) when you want to replace more complex objects according to conditions much more complicated than a simple arithmetic operation.

@lrhn
Copy link
Member

lrhn commented Jun 3, 2022

I'm with @natebosch too. This is too specialized, and not enough of an improvement vs. what you can already easily do.

The name would need tweaking ("replace" suggests destructive update) and we don't generally use named parameters like this.
Replacing with a single value is less flexible than allowing you to replace based on the element.

If I had to design it from scratch, I'd do:

extension ...<E> on Iterable<E> {
  Iterable<E> mapWhere(bool Function(E) test, E Function(E) convert) sync* {
    for (var element in this) yield test(element) ? convert(element) : element;
  }
}

But, at that point, foos.mapWhere(test, (e) => ....) could just be foos.map((e) => test(e) ? ... : e), and the latter is quite readable by itself. I don't think this method carries its own weight. Figuring out what it does and when to use it is more complication than just using .map.

Doesn't help that I can't come up with a situation where I'd want to use the function. If I have a list, I'd probably want to do the destructive update on it, instead of creating a new list.

So, regrettably, I'll say not to this addition. The code is fine, we're just trying to keep a balance between having functionality be available, so finding a solution is possible, and not having too many specialized solutions covering every individual use-case.

@lrhn lrhn closed this Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants