Scala Programming Language
  1. Scala Programming Language
  2. SI-3958

TraversableOnce requires more than foreach to be implemented

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Documentation and API
    • Labels:
      None

      Description

      The !TraversableOnce documentation says:

      This trait is composed of those methods which can be implemented solely in terms of foreach
      

      However, you have to implement many more methods than just foreach to get a !TraversableOnce:

      scala> val travOnce = new TraversableOnce[Int] { def foreach[U](f: Int => U) = f(1) }
      <console>:5: error: object creation impossible, since:
      method copyToArray in trait TraversableOnce of type [B >: Int](xs: Array[B],start: Int,len: Int)Unit is not defined
      method find in trait TraversableOnce of type (p: (Int) => Boolean)Option[Int] is not defined
      method exists in trait TraversableOnce of type (p: (Int) => Boolean)Boolean is not defined
      method forall in trait TraversableOnce of type (p: (Int) => Boolean)Boolean is not defined
      method toStream in trait TraversableOnce of type => Stream[Int] is not defined
      method toTraversable in trait TraversableOnce of type => Traversable[Int] is not defined
      method toIterator in trait TraversableOnce of type => Iterator[Int] is not defined
      method isTraversableAgain in trait TraversableOnce of type => Boolean is not defined
      method hasDefin...
             val travOnce = new TraversableOnce[Int] { def foreach[U](f: Int => U) = f(1) }
      

      I see that there's a comment above the forall declaration in !TraversableOnce.scala that says:

      Presently these are abstract because the Traversable versions use
      breakable/break, and I wasn't sure enough of how that's supposed to
      function to consolidate them with the Iterator versions.
      

      So I gather it's a known issue, but I didn't see an issue tracking this problem, and this is the third time that it's bitten me, so I figured I should report this.

      I guess the immediate solution for Scala 2.8 is to fix the documentation so that it says that many other things are required to define a !TraversableOnce, but it sure would be nice to only have to define foreach to get a !TraversableOnce like the documentation currently claims.

        Activity

        Hide
        Paul Phillips added a comment -

        It is presumably an errant search and replace making this claim in the documentation. It's not going to become true: the class which you can implement with foreach is Traversable.

        Show
        Paul Phillips added a comment - It is presumably an errant search and replace making this claim in the documentation. It's not going to become true: the class which you can implement with foreach is Traversable.
        Hide
        bethard added a comment -

        Ok, thanks for the clarification. I've switched this to a Documentation bug.

        It's too bad TraversableOnce isn't intended to work this way - it's much easier to describe control flow with the foreach method than with next and hasNext from Iterator. Compare the hypothetical TraversableOnce version of a restarting iterator:

        def restarting[T](getTrav: () => TraversableOnce[T]) = new TraversableOnce[T] {
          def foreach[U](f: T => U): Unit = {                                          
            try {                                                                      
              for (item <- getTrav())                                                  
                f(item)                                                                
            } catch {                                                                  
              case e: java.io.IOException => this.foreach(f)                           
            }                                                                          
          }                                                                            
        }
        

        with the Iterator implementation:

        def restarting[T](getIter: () => Iterator[T]) = new Iterator[T] {
          var iter = getIter()
          def hasNext = {
            try {
              iter.hasNext
            } catch {
              case e: IOException => {
                this.iter = getIter()
                this.hasNext
              }
            }
          }
          def next = {
            try {
              iter.next
            } catch {
              case e: IOException => {
                this.iter = getIter()
                this.next
              }
            }
          }
        }
        
        Show
        bethard added a comment - Ok, thanks for the clarification. I've switched this to a Documentation bug. It's too bad TraversableOnce isn't intended to work this way - it's much easier to describe control flow with the foreach method than with next and hasNext from Iterator . Compare the hypothetical TraversableOnce version of a restarting iterator: def restarting[T](getTrav: () => TraversableOnce[T]) = new TraversableOnce[T] { def foreach[U](f: T => U): Unit = { try { for (item <- getTrav()) f(item) } catch { case e: java.io.IOException => this.foreach(f) } } } with the Iterator implementation: def restarting[T](getIter: () => Iterator[T]) = new Iterator[T] { var iter = getIter() def hasNext = { try { iter.hasNext } catch { case e: IOException => { this.iter = getIter() this.hasNext } } } def next = { try { iter.next } catch { case e: IOException => { this.iter = getIter() this.next } } } }
        Hide
        Paul Phillips added a comment -

        You can as easily write that as

          new Traversable[T] { def foreach ... } iterator
        

        There are also methods like Iterator.iterate and Iterator.continually and more: you never have to define next and hasNext yourself unless you want to. TraversableOnce is a retrofit to eliminate code duplication between Iterator and Traversable, and except for internal use it's most likely less appropriate than one or the other of those.

        Show
        Paul Phillips added a comment - You can as easily write that as new Traversable[T] { def foreach ... } iterator There are also methods like Iterator.iterate and Iterator.continually and more: you never have to define next and hasNext yourself unless you want to. TraversableOnce is a retrofit to eliminate code duplication between Iterator and Traversable, and except for internal use it's most likely less appropriate than one or the other of those.
        Hide
        bethard added a comment -

        Replying to [comment:3 extempore]:
        > You can as easily write that as
        >


        > new Traversable[T]

        { def foreach ... }

        iterator
        > }}

        Except that the Traversable toIterator puts everything into a Buffer first, so you can't really use it if there are too many items in the iterator to keep them all around (e.g. an infinite stream read from a URL). If Traversable.iterator worked without building up the items, that would work perfectly though.

        > There are also methods like Iterator.iterate and Iterator.continually and more: you never have to define next and hasNext yourself unless you want to.

        But those don't lend themselves to handling exceptions. And yes I know about util.control, but I haven't yet seen a pretty solution to the code above using e.g. Iterator.continually and util.control. If you have one, it would be great if you could add it here:

        http://stackoverflow.com/questions/4010285/restart-iterator-on-exceptions-in-scala

        > TraversableOnce is a retrofit to eliminate code duplication between Iterator and Traversable, and except for internal use it's most likely less appropriate than one or the other of those.

        Maybe when the TraversableOnce docs are updated to fix the bug above, they could also say something about TraversableOnce being intended for internal use and link to the more appropriate methods?

        Show
        bethard added a comment - Replying to [comment:3 extempore] : > You can as easily write that as > > new Traversable [T] { def foreach ... } iterator > }} Except that the Traversable toIterator puts everything into a Buffer first, so you can't really use it if there are too many items in the iterator to keep them all around (e.g. an infinite stream read from a URL). If Traversable.iterator worked without building up the items, that would work perfectly though. > There are also methods like Iterator.iterate and Iterator.continually and more: you never have to define next and hasNext yourself unless you want to. But those don't lend themselves to handling exceptions. And yes I know about util.control, but I haven't yet seen a pretty solution to the code above using e.g. Iterator.continually and util.control. If you have one, it would be great if you could add it here: http://stackoverflow.com/questions/4010285/restart-iterator-on-exceptions-in-scala > TraversableOnce is a retrofit to eliminate code duplication between Iterator and Traversable, and except for internal use it's most likely less appropriate than one or the other of those. Maybe when the TraversableOnce docs are updated to fix the bug above, they could also say something about TraversableOnce being intended for internal use and link to the more appropriate methods?
        Hide
        Daniel Sobral added a comment -

        I don't see the problem here.

        Replying to [ticket:3958 bethard]:
        > The !TraversableOnce documentation says:
        >

        > This trait is composed of those methods which can be implemented solely in terms of foreach
        > }}
        
        > However, you have to implement many more methods than just {{foreach}} to get a !TraversableOnce:
        
        Ok. The question, therefore, is: can these many more methods be implemented in terms of foreach? If they can, the documentation is correct.
        
        The documentation DOES NOT SAY you just have to implement foreach. It says only that there isn't any methods that cannot be implemented in terms of foreach. Let's consider, for example, "exists", which is abstract. Here's an implementation:
        
        

        def exists(p: (A) ⇒ Boolean) : Boolean = {
        class ResultFound extends Exception
        var result = false
        try {
        foreach

        { el => result &&= p(el) if (result) throw new ResultFound }

        } catch (ex)

        { case _: ResultFound => }

        result
        }

        There, implemented in terms of foreach, as required. Alas, that sentence does not end there, and the text that follows it makes it more clear:

        > and which do not need access to a Builder

        Methods which need access to a Builder are those methods that return a collection of best fitting type. So there is no map method there, for example.

        Show
        Daniel Sobral added a comment - I don't see the problem here. Replying to [ticket:3958 bethard] : > The !TraversableOnce documentation says: > > This trait is composed of those methods which can be implemented solely in terms of foreach > }} > However, you have to implement many more methods than just {{foreach}} to get a !TraversableOnce: Ok. The question, therefore, is: can these many more methods be implemented in terms of foreach? If they can, the documentation is correct. The documentation DOES NOT SAY you just have to implement foreach. It says only that there isn't any methods that cannot be implemented in terms of foreach. Let's consider, for example, "exists", which is abstract. Here's an implementation: def exists(p: (A) ⇒ Boolean) : Boolean = { class ResultFound extends Exception var result = false try { foreach { el => result &&= p(el) if (result) throw new ResultFound } } catch (ex) { case _: ResultFound => } result } There, implemented in terms of foreach, as required. Alas, that sentence does not end there, and the text that follows it makes it more clear: > and which do not need access to a Builder Methods which need access to a Builder are those methods that return a collection of best fitting type. So there is no map method there, for example.
        Hide
        bethard added a comment -

        Maybe it's just a dialectal difference, but to me the sentence:

        This trait is composed of those methods which can be implemented
        solely in terms of foreach and which do not need access to a Builder.
        

        strongly suggests that all the methods that can be implemented in terms of foreach have been (and the last clause implies that no methods that require access to a builder are included in the TraversableOnce interface). But maybe that's just my dialect.

        How about the following suggested rewording that hopefully corresponds to the intention more clearly:

        This trait exists primarily to eliminate code duplication between
        Iterator and Traversable, and thus implements some of the common
        methods that can be implemented solely in terms of foreach without
        access to a Builder. It also includes a number of abstract methods
        whose implementations are provided by Iterator, Traversable, etc.
        
        Directly subclassing TraversableOnce is not recommended - instead,
        consider creating an Iterator with a next and hasNext method, or
        create an Iterator with one of the methods on the Iterator object.
        
        Show
        bethard added a comment - Maybe it's just a dialectal difference, but to me the sentence: This trait is composed of those methods which can be implemented solely in terms of foreach and which do not need access to a Builder. strongly suggests that all the methods that can be implemented in terms of foreach have been (and the last clause implies that no methods that require access to a builder are included in the TraversableOnce interface). But maybe that's just my dialect. How about the following suggested rewording that hopefully corresponds to the intention more clearly: This trait exists primarily to eliminate code duplication between Iterator and Traversable, and thus implements some of the common methods that can be implemented solely in terms of foreach without access to a Builder. It also includes a number of abstract methods whose implementations are provided by Iterator, Traversable, etc. Directly subclassing TraversableOnce is not recommended - instead, consider creating an Iterator with a next and hasNext method, or create an Iterator with one of the methods on the Iterator object.
        Hide
        Aleksandar Prokopec added a comment -

        (In r23538) Closes SI-3958. No review.

        Show
        Aleksandar Prokopec added a comment - (In r23538) Closes SI-3958 . No review.

          People

          • Assignee:
            Aleksandar Prokopec
            Reporter:
            bethard
            TracCC:
            Daniel Sobral, Paul Phillips
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development