Navigation Menu

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

Add predicate-parameterized search functions to List: indexOfMatch/lastIndexOfMatch #3711

Closed
kasperl opened this issue Jun 18, 2012 · 15 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-duplicate Closed in favor of an existing report P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@kasperl
Copy link

kasperl commented Jun 18, 2012

When we call indexOf/lastIndexOf on our list implementations, we let the elements in the list dictate if they are equals to the "thing" we're searching for. I find that a bit counterintuitive. I'd prefer if we always asked the thing we're searching for if it feels like it's equal to the elements like this:

class Yes { operator==(other) => true; }
var list = [ 1, 2, 3 ];
Expect.equals(0, list.indexOf(new Yes()));
Expect.equals(2, list.lastIndexOf(new Yes()));
@kasperl
Copy link
Author

kasperl commented Jun 18, 2012

Update bug description. Wooops.


Changed the title to: "Let the element we're looking for be the receiver in == calls in indexOf/lastIndexOf".

@lrhn
Copy link
Member

lrhn commented Jun 28, 2012

It's probably better that way, but mainly for performance reasons - it means that it's the same equals method that is called a lot of times, instead of potentially different ones called once each.

If the equality operator is done correctly, it should be symmetric, so it shouldn't matter which one is used. If the equality operator on your object cheats, and really just tries to match another object in some fancy way, you should search using a predicate instead (which we should make sure is also possible).

I actually slightly prefer using the existing objects' equality check in order to discourage such trickery. I.e., your example is a bad example. Positively naughty!

So I would suggest that List<T> gets
  int indexOfMatch(bool predicate(T value));
which you should use instead.

@DartBot
Copy link

DartBot commented Jun 30, 2012

This comment was originally written by @seaneagan


whichever one is the receiver, it should be consistent with Collection#contains (issue #1030).

Instead of List#indexOfMatch/indexOfLastMatch I would prefer Collection#find:

E find(bool predicate(E item));

@DartBot
Copy link

DartBot commented Jul 1, 2012

This comment was originally written by @seaneagan


I filed Collection#find as issue #3948

@lrhn
Copy link
Member

lrhn commented Sep 18, 2012

I think we should have both List.indexOfMatch/lastIndexOfMatch and Collection.find. The collection method makes sense for all collections, but index-operations only make sense for lists.


Added this to the M2 milestone.
Added Accepted label.

@lrhn
Copy link
Member

lrhn commented Oct 24, 2012

It is consistent with Collection.contains now. We generally let the collection determine if it has something equal to the argument of contains, indexOf (and if we don't, we should fix that).

@DartBot
Copy link

DartBot commented Dec 6, 2012

This comment was originally written by @seaneagan


If we had List.pairs (see issue #3948) and Iterable.find (issue #7088), then we probably wouldn't need indexOfMatch/indexOfLastMatch since then instead of:

list.indexOfMatch(test)

one could do:

list.pairs.find((pair) => test(pair.value)).key;

... which also allows one to find both the matching index and value with a single iteration of the list.

@floitschG
Copy link
Contributor

Removed this from the M2 milestone.
Added this to the M3 milestone.

@anders-sandholm
Copy link
Contributor

Removed this from the M3 milestone.
Added this to the M4 milestone.

@larsbak
Copy link

larsbak commented May 28, 2013

Removed this from the M4 milestone.
Added this to the M5 milestone.

@lrhn
Copy link
Member

lrhn commented Aug 19, 2013

We are not going to change the order of the operands of ==.
I'll keep this issue open as a request for indexOfMatch/lastIndexOfMatch.


Removed Type-Defect label.
Added Type-Enhancement label.
Changed the title to: "Add predicate-parameterized search functions to List: indexOfMatch/lastIndexOfMatch".

@kasperl
Copy link
Author

kasperl commented Jun 4, 2014

Removed this from the M5 milestone.
Added this to the 1.6 milestone.

@kasperl
Copy link
Author

kasperl commented Jul 10, 2014

Removed this from the 1.6 milestone.
Added Oldschool-Milestone-1.6 label.

@kasperl
Copy link
Author

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-1.6 label.

@kasperl kasperl added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Aug 4, 2014
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed accepted labels Feb 29, 2016
@lrhn lrhn added the core-m label Aug 11, 2017
@floitschG
Copy link
Contributor

Closing as dupe of #30275.

@floitschG floitschG added closed-duplicate Closed in favor of an existing report and removed core-m labels Aug 14, 2017
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-duplicate Closed in favor of an existing report P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants