tweaks in response to NthPortal review

Author: SethTisue

src/main/scala/scala/collection/immutable/next/package.scala

@@ -37,8 +37,6 @@ package object next {
       // `LazyList.empty #:::` accomplishes that delay
       else LazyList.empty #::: {
         // case 2: the input is later discovered to be empty
-        // (distinguishing this from case 3 isn't necessary for correctness, but we
-        // might as well avoid the lazy val path if we can)
         if (ll.isEmpty) LazyList.empty
         else {
           // case 3: non-empty

src/test/scala/scala/collection/immutable/TestLazyListExtensions.scala

@@ -20,37 +20,41 @@ import next._
 class TestLazyListExtensions {
 
   // This method will *not* terminate for non-cyclic infinite-sized collections.
+  // (It's kind of nasty to have tests whose failure mode is to hang, but I don't
+  // see an obvious alternative that doesn't involve copying code from LazyList.
+  // Perhaps this could be improved at the time this all gets merged into stdlib.)
   def assertConstantMemory[T](xs: LazyList[T]): Unit =
     // `force` does cycle detection, so if this terminates, the collection is
     // either finite or a cycle
     xs.force
 
   @Test
-  def testEmpty1(): Unit = {
+  def cycleEmpty1(): Unit = {
     val xs = LazyList.empty  // realized
     val cyc = xs.cycle
     assertTrue(cyc.isEmpty)
     assertTrue(cyc.size == 0)
     assertEquals(Nil, cyc.toList)
   }
   @Test
-  def testEmpty2(): Unit = {
-    val xs = LazyList()  // not realized
+  def cycleEmpty2(): Unit = {
+    val xs = LazyList.empty #::: LazyList.empty  // not realized
+    assertEquals(-1, xs.knownSize)  // double-check it's not realized
     val cyc = xs.cycle
     assertTrue(cyc.isEmpty)
     assertTrue(cyc.size == 0)
     assertEquals(Nil, cyc.toList)
   }
   @Test
-  def testNonEmpty(): Unit = {
+  def cycleNonEmpty(): Unit = {
     val xs = LazyList(1, 2, 3)
     val cyc = xs.cycle
     assertFalse(cyc.isEmpty)
     assertConstantMemory(cyc)
     assertEquals(LazyList(1, 2, 3, 1, 2, 3, 1, 2), cyc.take(8))
   }
   @Test
-  def testToString(): Unit = {
+  def cycleToString(): Unit = {
     assertEquals("LazyList()",
       LazyList.empty.cycle.toString)
     assertEquals("LazyList(<not computed>)",
@@ -60,22 +64,22 @@ class TestLazyListExtensions {
       LazyList(1, 2, 3).cycle.force.toString)
   }
   @Test
-  def testRepeats(): Unit = {
+  def cycleRepeats(): Unit = {
     val xs = LazyList(1, 2, 3)
     val cyc = xs.cycle
     assertFalse(cyc.isEmpty)
     assertEquals(LazyList(1, 2, 3, 1, 2, 3, 1, 2), cyc.take(8))
   }
   @Test
-  def testConstantMemory1(): Unit = {
+  def cycleConstantMemory1(): Unit = {
     val xs = LazyList(1, 2, 3)
     val cyc = xs.cycle
     assertTrue(cyc.tail eq cyc.tail.tail.tail.tail)
     assertTrue(cyc.tail.tail eq cyc.drop(4).tail)
     assertTrue(cyc.tail eq cyc.drop(3).tail)
   }
   @Test
-  def testConstantMemory2(): Unit = {
+  def cycleConstantMemory2(): Unit = {
     var counter = 0
     def count(): Int = { counter += 1; counter }
     val xs = count() #:: count() #:: count() #:: LazyList.empty
@@ -85,7 +89,7 @@ class TestLazyListExtensions {
     assertEquals(3, counter)
   }
   @Test
-  def testConstantMemory3(): Unit = {
+  def cycleConstantMemory3(): Unit = {
     val xs = LazyList(1, 2, 3)
     val cyc = xs.cycle
     assertConstantMemory(cyc)
@@ -97,19 +101,20 @@ class TestLazyListExtensions {
     assertConstantMemory(cyc.drop(10))
   }
   @Test
-  def testUnbounded(): Unit = {
+  def cycleUnbounded(): Unit = {
     val xs = LazyList.from(1)
     val cyc = xs.cycle
     assertEquals(LazyList(1, 2, 3), cyc.take(3))
   }
   @Test
-  def testSecondCallIsSafeButNotIdempotent(): Unit = {
+  def cycleSecondCallIsSafeButNotIdempotent(): Unit = {
     val xs = LazyList(1, 2, 3)
     // this is safe to do
     val twice = xs.cycle.cycle
     // and the contents are as expected
     assertEquals(LazyList(1, 2, 3, 1, 2, 3, 1, 2), twice.take(8))
-    // but the result is not a cycle. TODO do we care?
+    // but the result is not a cycle. it might be nice if it were, but oh well.
+    // testing the existing behavior.
     assertFalse(twice.tail eq twice.tail.tail.tail.tail)
   }
 }