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

concept of List.indexWhere functionality #30275

Closed
fallenreaper opened this issue Jul 26, 2017 · 15 comments
Closed

concept of List.indexWhere functionality #30275

fallenreaper opened this issue Jul 26, 2017 · 15 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 library-core type-enhancement A request for a change that isn't a bug

Comments

@fallenreaper
Copy link

fallenreaper commented Jul 26, 2017

As a consumer of the dartlang, it seems that more often than not I am given an object with some primary key attributes, such as an ID and want to get that Index to deal with CSS or set flags. Right now, the use cases I have seen is either:

    int targetId = 5
    var item = list.firstWhere((obj)=>obj.id == targetId, orElse: ()=>null;
    var index = item==null? -1 : list.indexOf(item);

or:

    int index = 0;
    bool found = false;
    for (var item in list) {
      if (item.id == targetId) {
        found = true;
        break;
       }
       index++;
    }
    return found ? index : -1;

I have corrected my developers for using an ineffecient case (top) but thought it would be useful to implement a function for lists called: indexWhere

    int indexWhere(func, {orElse: ()=> -1){
      int index = 0;
      bool found = false;
      for (var item in list){
        if (func(item)){
          found = true;
          break;
        }
        index ++;
      }
      return found ? index : fn();
    }
@a-siva a-siva added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Jul 26, 2017
@lrhn lrhn added library-core type-enhancement A request for a change that isn't a bug labels Jul 30, 2017
@lrhn
Copy link
Member

lrhn commented Jul 30, 2017

This would make sense as a method on List.
I don't think it's as useful on Iterable in general since the index is less important there (you can use elementAt, but you shouldn't do that a lot if it's not actually a list).

@fallenreaper
Copy link
Author

fallenreaper commented Jul 30, 2017 via email

@lrhn
Copy link
Member

lrhn commented Aug 11, 2017

See also #3711

@floitschG
Copy link
Contributor

floitschG commented Aug 14, 2017

It should only be on List.
We should probably also make the signature match the one from indexOf: int indexWhere(func, [int start = 0]).

It should be relatively easy to go from -1 to the value the user wants.

@srawlins
Copy link
Member

@lrhn
Copy link
Member

lrhn commented Apr 13, 2018

It did, yes. Thanks.

@dotdoom
Copy link

dotdoom commented Dec 14, 2019

If this ever gets any reconsideration, my 2cents: it could be useful on Iterator, too. I believe it's not different implementation-wise between List and Iterable, but BuiltList extends Iterable and not List.

Now with extension methods, if I want to add indexById() extension method on both List and BuiltList, I have to add it to Iterable and can't use indexWhere.

@fallenreaper
Copy link
Author

fallenreaper commented Dec 14, 2019 via email

@lrhn
Copy link
Member

lrhn commented Dec 15, 2019

It is very unlikely that a change to the Iterable class would be able to land at this time. It would break all existing implementations of Iterable that are not also a List.

(I also do not recommend indexing into Iterables by integers, it's very rarely going to be efficient).

@fallenreaper
Copy link
Author

fallenreaper commented Dec 15, 2019 via email

@fallenreaper
Copy link
Author

@dotdoom i was looking at the DartLang docs, and this was implemented as a part of the the List class. It does not make sense for iterable as not all iterables are indexed like Lists are. There are likely other classes which would make use of indexWhere function, but to me, adding them as a mixin are also not the right way. I think that if there are other classes which may benefit from this, submit requests and someone will get to them.

@dotdoom
Copy link

dotdoom commented Dec 18, 2019

@fallenreaper I don't see how it will hurt either (elementAt already paves the way for indexing Iterables), but it will clearly be beneficial for solving the problem I mentioned above.

@dotdoom
Copy link

dotdoom commented Dec 18, 2019

@lrhn sure there are other methods that may be slow on certain implementations of iterable -- such as length, last etc. Iterables are full of catches, such as a single-shot iterable has to be converted to a list to take a second pass. That doesn't mean however that we should remove all those methods and keep only moveNext. The methods are there for convenience, and convenience is always a tradeoff.

Don't want to make a big deal out of it; I wanted to say that adding it to Iterable would solve one problem without introducing any other, that's all.

@fallenreaper
Copy link
Author

@dotdoom I was looking at the recent implementation Iterable. You are actually correct, good call. I dont think that was a part of iterable when I was doing dart back then. I was looking at the underlying implementation of elementAt for example, and it starts at the beginning of the Iterable and walks in until index is determined, so unlike real implementation of Lists where access is O(1), access on an iterable is O(index). So it isnt all that good to me.

That being said, indexWhere, while it may make use because elementAt and firstWhere are implemented, after getting the index, accessing it through elementAt would still take time to access.

indexWhere, Runtime => O(N) to find the index.
elementAt, Runtime => O(N) to find the object BECAUSE elementAt is not O(1) access.
So, doing these will create issues of O(2*N) which isnt all that good in comparison to other options.

@dotdoom
Copy link

dotdoom commented Dec 18, 2019

@fallenreaper indeed; however, consider that (and now I'm feel like I'm derailing the conversation too much)

  • indexWhere() does not necessarily imply subsequent elementAt(); there are use cases where it's good to know the index (e.g. to count number of elements before a specific one)
  • the calculations you have suggested are valid only for implementations that require sequential access, such as linked list; other implementations may win (vector, b-tree) or loose (computational, such as an iterable over digits of Pi) in performance.

P.S. O(2*N) = O(N) ;-)

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. core-2 library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants