From 53f81d1f418850ad8c9935d0dd79fdfb9cc84b44 Mon Sep 17 00:00:00 2001
From: Frank Rosner <frank@fam-rosner.de>
Date: Sat, 25 Nov 2017 23:33:40 +0100
Subject: [PATCH 1/2] Replace nondeterministic performantTraversables

The `performantTraversables` exercise has three problems:

1. It is not deterministic. People report (and I experienced it myself) that the result is sometime `true` and sometimes `false`.
2. You should not benchmark method calls like this. The JVM needs to warm up, there are a lot of optimizations going on. Use a microbenchmarking framework.
3. The comment states that you might receive a `StackOverflowError`. This is not true, as `reduceRight` is implemented based on `reduceLeft`, making it also tail recursive if `reduceLeft` is tail recursive.

I suggest to replace the exercise with another one demonstrating the relationship between the left and the right operation. The new exercise is deterministic. Also I removed the wrong comment about the `StackOverflowError`.
---
 src/main/scala/stdlib/Traversables.scala | 28 ++++++++++--------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/main/scala/stdlib/Traversables.scala b/src/main/scala/stdlib/Traversables.scala
index 3027496c..70fa4605 100644
--- a/src/main/scala/stdlib/Traversables.scala
+++ b/src/main/scala/stdlib/Traversables.scala
@@ -511,23 +511,19 @@ object Traversables extends FlatSpec with Matchers with org.scalaexercises.defin
     intList.min should be(res3)
   }
 
-  /** You would choose `foldLeft`/`reduceLeft` or `foldRight`/`reduceRight` based on your mathematical goal. One other reason for deciding is performance - `foldLeft` generally has better performance since it uses tail recursion. This exercise will either work fine or you will receive a `StackOverflowError`:
+  /** The naive recursive implementation of `reduceRight` is not tail recursive and would lead to a stack overflow if used on larger traversables.
+   * However, `reduceLeft` can be implemented with tail recursion.
+   *
+   * To avoid the potential stack overflow with the naive implementation of `reduceRight` we can easily implement it based on `reduceLeft` by reverting the list and the inverting the reduce function. 
+   * The same applies for folding operations. 
+   *
+   * There is also a `reduce` (and `fold`) available, which works exactly like `reduceLeft` (and `foldLeft`) and it should be the prefered method to call unless there is a strong reason to use `reduceRight` (or `foldRight`).
    */
-  def performantTraversables(res0: Boolean) {
-    val MAX_SIZE            = 1000000
-    val reduceLeftStartTime = new java.util.Date
-    (1 to MAX_SIZE) reduceLeft (_ + _)
-    val reduceLeftEndTime = new java.util.Date
-
-    val reduceRightStartTime = new java.util.Date
-    (1 to MAX_SIZE) reduceRight (_ + _)
-    val reduceRightEndTime = new java.util.Date
-
-    val totalReduceLeftTime = reduceLeftEndTime.getTime - reduceLeftStartTime.getTime
-    val totalReduceRightTime = reduceRightEndTime.getTime - reduceRightStartTime
-      .getTime
-
-    (totalReduceRightTime > totalReduceLeftTime) should be(res0)
+  def reduceRightAsReduceLeft(res0: Int, res1: Int, res2: Int) {
+    val intList = List(5, 4, 3, 2, 1)
+    intList.reduceRight((x, y) => x - y) should be(res0)
+    intList.reverse.reduceLeft((x, y) => y - x) should be(res1)
+    intList.reverse.reduce((x, y) => y - x) should be(res2)
   }
 
   /** `transpose` will take a traversable of traversables and group them by their position in it's own traversable, e.g.:

From 48d78030a19364a27914298df7c3280b55229192 Mon Sep 17 00:00:00 2001
From: Frank Rosner <frank@fam-rosner.de>
Date: Sat, 25 Nov 2017 23:55:23 +0100
Subject: [PATCH 2/2] replace FIXME test with a real one

---
 src/test/scala/stdlib/TraversablesSpec.scala | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/test/scala/stdlib/TraversablesSpec.scala b/src/test/scala/stdlib/TraversablesSpec.scala
index 3ebf8f51..14334e3e 100644
--- a/src/test/scala/stdlib/TraversablesSpec.scala
+++ b/src/test/scala/stdlib/TraversablesSpec.scala
@@ -441,15 +441,14 @@ class TraversablesSpec extends Spec with Checkers {
     )
   }
 
-  // FIXME: this test depends on runtime timing of code
-  // def `performant traversals` = {
-  //   check(
-  //     Test.testSuccess(
-  //       Traversables.performantTraversables _,
-  //       false :: HNil
-  //     )
-  //   )
-  // }
+  def `reduceRight as reduceLeft` = {
+    check(
+      Test.testSuccess(
+        Traversables.reduceRightAsReduceLeft _,
+        3 :: 3 :: 3 :: HNil
+      )
+    )
+  }
 
   def `transpose function` = {
     check(