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

Two-argument indexOf does not work for Iterator #8552

Closed
scabug opened this issue May 1, 2014 · 8 comments
Closed

Two-argument indexOf does not work for Iterator #8552

scabug opened this issue May 1, 2014 · 8 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented May 1, 2014

Iterator(5).indexOf(5,0) does not work. In fact, every call to the two-arg version of indexOf seems to return -1. I presume the iterator is being consumed completely instead of partially.

@scabug
Copy link
Author

scabug commented May 1, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8552?orig=1
Reporter: @Ichoran
Affected Versions: 2.11.0

@scabug
Copy link
Author

scabug commented May 17, 2014

@ruippeixotog said (edited on May 17, 2014 10:18:25 PM UTC):
It seems Iterator doesn't have a two-arg version of indexOf. The two arg version present in other collections comes from GenSeqLike, which Iterator does not implement:

def indexOf[B >: A](elem: B, from: Int): Int = indexWhere(elem == _, from)

When indexOf is called on an Iterator with two arguments, the compiler is converting it to a call to the one-arg version with a tuple argument:

scala> Iterator(5 -> 0).indexOf(5,0)
res0: Int = 0

@scabug
Copy link
Author

scabug commented May 29, 2014

@Ichoran said:
Ah, good catch. It's still a bug, I think, to not have it (especially since it's trivially implementable), but since it's not there I can't fix it in 2.11 (binary & source compatibility issues).

@scabug
Copy link
Author

scabug commented Apr 4, 2015

@ruippeixotog said:
@Ichoran I'll add the new method to Iterator and create a pull request for 2.12.x. Is that ok with you?

@scabug
Copy link
Author

scabug commented Apr 4, 2015

@ruippeixotog said:
Opened up a pull request for this at scala/scala#4429.

@scabug
Copy link
Author

scabug commented Apr 16, 2015

Kevin Meredith (kevinmeredith) said (edited on Apr 16, 2015 4:49:08 PM UTC):
Note - I have not contributed to scala or the collections library at all.

I've noticed that other Iterator.scala code, as well as your PR, @rui, included var's.

Why not implement indexWhere in a functional way? Something like:

  override def indexWhere[B >: A](p: A => Boolean, from: Int): Int = 
   self.zipWithIndex.drop(from).find(p(_._1)) match {
      case None             => -1
      case Some((_, index)) => index
    } 

Note - I don't know if self is correct here. But this REPL examples shows a few simple examples working:

scala> Iterator(1,2,3).zipWithIndex.drop(0).find(_._1 == 3)
res2: Option[(Int, Int)] = Some((3,2))

@scabug
Copy link
Author

scabug commented Apr 16, 2015

@Ichoran said:
[~kevinmeredith] You wouldn't do it that way because that implementation is much slower, and core library functions ought not assume that the user doesn't care how fast they are. You could write a tail-recursive function, but a simple loop with a mutable var isn't that hard to understand. The var is completely contained and the logic is straightforward.

@scabug
Copy link
Author

scabug commented Apr 30, 2015

@Ichoran said:
scala/scala#4429

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

No branches or pull requests

2 participants