How do I implement an iterator for an existing singly linked list?

Question

I am trying to implement an iterable for this singly linked list using dependency injection. I am expecting hasNext to return true and next() to return the first element. But it errors out. What am I doing wrong here?

Thanks for your help in advance.

// Definition for singly-linked list from codefights:
class ListNode[T](x : T) {
  var value: T = x
  var next: Option[ListNode[T]] = None
}
//This class will provide iterator for the above list.    
implicit class ln(lns: Option[ListNode[Int]]) extends Iterable[Option[ListNode[Int]]] {

  var ptr = lns  //temporary variable to store the state of iterator

  override def iterator = new Iterator[Option[ListNode[Int]]] {
    //if the current node pointed to is None, there are no elements.
    override def hasNext = ptr match {case None=> false; case _ => true}

    //store the current ptr to temp variable and return it.
    override def next = { var tmp = ptr; ptr = ptr.get.next; tmp }
  }

  //defined +: to perform tests
  def +:(that: Option[ListNode[Int]]) = that match {
    case None => lns
    case _ => that.get.next = lns; that
  }


}

//create 3 nodes that will be linked in next step
var a = new ListNode(1)
var b = new ListNode(2)
var c = new ListNode(3)


//link the 3 nodes
val xx : Option[ListNode[Int]]= Some(c) +: Some(b) +: Some(a)

//I know this is not dependency injection. But I wanted to debug the issue.
val ax: ln = new ln(xx)
//ax.size

//checking the iterator manually
ax.iterator.hasNext
ax.iterator.next()

Output:

> defined class ListNode
> defined class ln
> a: ListNode[Int] = ListNode@79c41e43 b: ListNode[Int] =
> ListNode@5fbdb591 c: ListNode[Int] = ListNode@a8e723
> 
> xx: Option[ListNode[Int]] = Some(A$A131$A$A131$ListNode@a8e723)
> 
> ax: ln = A.A131.A.A131(Some(A$A131$A$A131$ListNode@a8e723),
> Some(A$A131$A$A131$ListNode@5fbdb591),
> Some(A$A131$A$A131$ListNode@79c41e43))
> 
> res0: Boolean = false java.util.NoSuchElementException: None.get  at
> scala.None$.get(palindrom.sc:345)     at
> scala.None$.get(palindrom.sc:343)     at
> #worksheet#.ln$$anon$1.next(palindrom.sc:23)  at 

#worksheet#.ln$$anon$1.next(palindrom.sc:20)    at #worksheet#.get$$instance$$res1(palindrom.sc:39)     at #worksheet#.#worksheet#(palindrom.sc:84)

Update: I am not allowed to modify class ListNode. Which I did not mention before. My mistake. I just added toString below for clear printing though.

But I modified the rest of the code based on @Dima suggestion. I am wowed!! Because, I can just use map like a regular list. Thanks a lot!!

class ListNode[T](x : T) {
  var value: T = x
  var next: Option[ListNode[T]] = None
  override def toString = x.toString
}



implicit class ln(val lns: ListNode[Int]) extends  Iterable[Int] {

  override def iterator = Iterator.iterate(Option(lns))(_.flatMap(_.next))
    .takeWhile(_.nonEmpty)
    .flatten
    .map(_.value)

  def +:(that: Int) : ListNode[Int] = {
    val newNode = new ListNode(that)
      newNode.next = Some(lns)
      newNode}


}


val xx : ListNode[Int]= 1 +: 2 +: new ListNode(3)

xx.foreach(println)

xx.map( _ +1)

Output

xx: ListNode[Int] = 1

res0: Iterable[Int] = List(2, 3, 4)

Dima · Accepted Answer · 2017-07-01T21:52:59.640

Don't do this. Just don't. I am sorry, I started typing several times trying to explain what's wrong with it, but I can't ... it's all wrong.

Back to the drawing board. First of all, scrap the mutable state. You don't need it. For now, just pretend, that there is no var in scala.

case class ListNode[T](value: T, next: Option[ListNode[T]] = None) {
   def +:(head: T) = ListNode(head, Some(this))
}

Now. It seems weird, that your iterator is iterating over Option[ListNode[T]] rather than T, or, at least, ListNode[T]. Wrapping it into Option seems completely useless. This iterates over T, because it seems the most sensible, if you want ListNode[T] instead, remove the last .map. If you insist on Option[ListNode[T]], remove both .map and .flatten.

object ListNode {
   implicit class Iterable[T](val node: ListNode[T]) extends AnyVal {
      def iterator = Iterator
         .iterate(Option(node))(_.flatMap(_.next))
         .takeWhile(_.nonEmpty)
         .flatten
         .map(_.value)
   }
}

Now you can do something like:

 (1 +: 2 +: ListNode(3))
   .iterator
   .toList // returns List(1,2,3)

This is it. If you are going to write code in scala, you might as well take a few minutes to learn how the language is intended to be used. You'll grow to appreciate it.

How do I implement an iterator for an existing singly linked list?

1 Answers1

Linked