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

methods such as removeWhere, retainWhere etc. should return information #15703

Closed
DartBot opened this issue Dec 18, 2013 · 3 comments
Closed
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant core-n library-core P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Dec 18, 2013

This issue was originally filed by @xximranxx


The list function removeWhere, retainWhere etc. are great functions.

But they return void.

It would be even better if they returned e.g. the number of elements removed or retained etc.

Lets say I want to remove certain elements in a list and if I did, then do something else something else.
At the moment, I would have to to firstWhere to check whether any element matches the criteria and then removeWhere.

Please correct me if this is already possible and I missed it

Thanks,
Imran

@floitschG
Copy link
Contributor

Added Area-Library, Triaged labels.

@lrhn
Copy link
Member

lrhn commented Dec 19, 2013

Returning information that is otherwise available is a two-edged sword.
It may make some things slightly simpler, but it is also an overhead in the cases where the information isn't needed.
And it risks not being the thing you actually need. Something that makes perfect sense in one case might not make sense in another, depending on what your collection represents. We could return a boolean that was true if something changed, and false if not (as we do for remove), but if you need the count, that is just unusable overhead.

In many cases, encapsulating the collection in a more domain related class will allow you to do the thing that makes sense, and it doesn't matter that you use a few more lines in the abstraction if it makes the use of it simple. It does matter to use a few more lines in the basic collection, because it hits every use of it.

In this case, the number of elements removed is reflected in the length of the container, so you can do something like:
  if (set.length > (set..retainAll(something)).length) // something changed
or
  if (set.length - (set..removeAll(something)).length == something.length) {
    // removed all
  }
It does look a little confusing, and I can see the wish for shorter code, but I don't have any current plans for it.


Removed Type-Defect, Priority-Unassigned labels.
Added Type-Enhancement, Priority-Low labels.

@DartBot
Copy link
Author

DartBot commented Dec 19, 2013

This comment was originally written by @xximranxx


Yes, fully agreed that overhead should be avoided in basic collection.

Actually, in my specific use-case, I would only need the boolean return whether anything was affected or not.

If returning that information does not cost overhead, I think it would be a useful change in code.

Thanks

@DartBot DartBot added Type-Enhancement P3 A lower priority bug or feature request area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Dec 19, 2013
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed triaged labels Mar 1, 2016
@floitschG floitschG added core-n and removed core-m labels Aug 28, 2017
@matanlurey matanlurey added the closed-obsolete Closed as the reported issue is no longer relevant label Jun 19, 2018
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-obsolete Closed as the reported issue is no longer relevant core-n library-core P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants