Refactoring to support parsing choices safely

- Sayable.Group, to preserve a grouping
- Make Choice an ADT, and move it to top level
- Move assigning logic into ChoiceMenu and move it to top level
- Factor out choiceMenuSayable
- Tested ParseChoices

core/src/main/scala/simpleivr/Choice.scala

@@ -0,0 +1,23 @@
+package simpleivr
+
+sealed trait Choice[+A] {
+  def maybeDtmf: Option[DTMF]
+  def label: Sayable
+  def value: A
+  def map[B](f: A => B): Choice[B]
+}
+
+object Choice {
+  case class Unassigned[+A](label: Sayable, value: A) extends Choice[A] {
+    override def maybeDtmf = None
+    override def map[B](f: A => B) = copy(value = f(value))
+  }
+  case class Assigned[+A](dtmf: DTMF, label: Sayable, value: A) extends Choice[A] {
+    override def maybeDtmf = Some(dtmf)
+    override def map[B](f: A => B) = copy(value = f(value))
+  }
+  def apply[A](label: Sayable, value: A): Choice[A] = Unassigned(label, value)
+  def apply[A](dtmf: DTMF, label: Sayable, value: A): Choice[A] = Assigned(dtmf, label, value)
+
+  def unapply[A](choice: Choice[A]) = Some((choice.maybeDtmf, choice.label, choice.value))
+}

core/src/main/scala/simpleivr/ChoiceMenu.scala

@@ -0,0 +1,23 @@
+package simpleivr
+
+case class ChoiceMenu[A](title: Sayable, choices: Seq[Choice[A]]) {
+  private def assignNums(choices: List[Choice[A]], nums: List[DTMF]): List[Choice.Assigned[A]] = choices match {
+    case Nil                                       => Nil
+    case (c @ Choice.Assigned(dtmf, _, _)) :: rest => c :: assignNums(rest, nums.filter(_ != dtmf))
+    case Choice.Unassigned(label, value) :: rest   =>
+      if (nums.nonEmpty)
+        Choice.Assigned(nums.head, label, value) :: assignNums(rest, nums.tail)
+      else {
+        println("ERROR: No num available for choices: " + choices)
+        assignNums(rest, nums)
+      }
+  }
+
+  lazy val assigned =
+    assignNums(choices.toList, DTMF.digits.toList.filterNot(n => choices exists (_.maybeDtmf contains n)))
+}
+
+object ChoiceMenu {
+  def apply[A](title: Sayable, dummy: Null = null)(choices: Choice[A]*): ChoiceMenu[A] =
+    new ChoiceMenu(title, choices)
+}

core/src/main/scala/simpleivr/DefaultSayer.scala

@@ -32,6 +32,8 @@ class DefaultSayer(interp: IvrCommand.Interpreter[IO], interruptDtmfs: Set[DTMF]
       .flatMap(play)
   }
 
+  override def group(sayable: Sayable) = apply(sayable)
+
   override def many(sayables: List[Sayable]): IO[Option[DTMF]] =
     sayables match {
       case Nil         => IO.pure(None)

core/src/main/scala/simpleivr/IvrChoices.scala

@@ -9,14 +9,6 @@ class IvrChoices(sayables: Sayables) extends Ivr(sayables) {
   import sayables._
 
 
-  case class Choice[+A](key: Option[DTMF], label: Sayable, value: A) {
-    def map[B](f: A => B): Choice[B] = Choice(key, label, f(value))
-  }
-  object Choice {
-    def apply[A](label: Sayable, value: A): Choice[A] = new Choice(None, label, value)
-    def apply[A](key: DTMF, label: Sayable, value: A): Choice[A] = new Choice(Some(key), label, value)
-  }
-
   def paginated[T](maximum: Int, choices: List[Choice[T]], fixedChoices: List[Choice[T]]): List[Choice[IvrStep[T]]] = {
     def doPage(page: List[Choice[T]], rest: List[Choice[T]], back: List[Choice[T]]): List[Choice[IvrStep[T]]] = {
       def askNextPage: IvrStep[T] = Free.defer {
@@ -47,57 +39,39 @@ class IvrChoices(sayables: Sayables) extends Ivr(sayables) {
     doPage(choices.take(maximum - 1), choices.drop(maximum - 1), Nil)
   }
 
-  def assignNums[A](choices: List[Choice[A]]): List[Choice[A]] = {
-    val unused = DTMF.digits.toList.filterNot(n => choices exists (_.key contains n))
-    println("assignNums: unused: " + unused.mkString)
-    for (c <- choices) println(s"  ${c.key} / ${c.label}")
-    assignNums[A](choices, unused)
-  }
-  def assignNums[A](choices: List[Choice[A]], nums: List[DTMF]): List[Choice[A]] = choices match {
-    case Nil                                 => Nil
-    case (c @ Choice(Some(n), _, _)) :: rest => c :: assignNums(rest, nums.filter(_ != n))
-    case c :: rest                           =>
-      if (nums.nonEmpty)
-        c.copy(key = Some(nums.head)) :: assignNums(rest, nums.tail)
-      else {
-        println("ERROR: No num available for choices: " + choices)
-        c :: rest
-      }
-  }
-
-  case class ChoiceMenu[A](title: Sayable, choices: Seq[Choice[A]])
-  object ChoiceMenu {
-    def apply[A](title: Sayable, dummy: Null = null)(choices: Choice[A]*): ChoiceMenu[A] =
-      new ChoiceMenu(title, choices)
-  }
-
   sealed class SayChoice(val func: (DTMF, Sayable) => Sayable)
   object SayChoice {
     case object LabelFirst extends SayChoice((key, label) => label & `Press` & dtmfWord(key))
     case object LabelLast extends SayChoice((key, label) => `Press` & dtmfWord(key) & label)
   }
 
+  def choiceMenuSayable[A](choiceMenu: ChoiceMenu[A], pauseMs: Int, sayChoice: SayChoice) =
+    choiceMenu.title &
+      Sayable.Many(
+        choiceMenu.assigned.map {
+          case Choice.Assigned(key, label, _) =>
+            Sayable.Group(Pause(pauseMs) & sayChoice.func(key, label))
+        }
+      )
+
   protected def defaultAskChoicePauseMs: Int = 750
   protected def defaultAskChoiceSayChoice: SayChoice = SayChoice.LabelLast
 
   def askChoice[A](choiceMenu: ChoiceMenu[A],
                    pauseMs: Int = defaultAskChoicePauseMs,
                    sayChoice: SayChoice = defaultAskChoiceSayChoice): IvrStep[A] = {
-    val menu = assignNums(choiceMenu.choices.toList)
-    val menuMsgs = menu.collect {
-      case Choice(Some(key), label, _) => Pause(pauseMs) & sayChoice.func(key, label)
-    }
-    if (menuMsgs.length < menu.length)
-      Console.err.println(s"ERROR: Not all menu choices have keys in ${choiceMenu.title}: $menu")
-
-    def loop: IvrStep[A] = sayAndGetDigit(choiceMenu.title & Sayable.Many(menuMsgs)) flatMap {
-      case None    => IvrStep.say(`Please make a selection` & Pause(pauseMs)) *> loop
-      case Some(c) =>
-        menu.find(_.key.contains(c)) match {
-          case Some(choice) => IvrStep(choice.value)
-          case None         => IvrStep.say(`That is not one of the choices.` & Pause(pauseMs)) *> loop
+    val menuSayable = choiceMenuSayable(choiceMenu, pauseMs, sayChoice)
+
+    def loop: IvrStep[A] =
+      sayAndGetDigit(menuSayable)
+        .flatMap {
+          case None    => IvrStep.say(`Please make a selection` & Pause(pauseMs)) *> loop
+          case Some(c) =>
+            choiceMenu.assigned.find(_.maybeDtmf.contains(c)) match {
+              case Some(choice) => IvrStep(choice.value)
+              case None         => IvrStep.say(`That is not one of the choices.` & Pause(pauseMs)) *> loop
+            }
         }
-    }
 
     loop
   }

core/src/main/scala/simpleivr/Sayable.scala

@@ -18,6 +18,7 @@ sealed trait Sayable {
   final def toSingles: Seq[Sayable.Single] = this match {
     case single: Sayable.Single => Seq(single)
     case Sayable.Many(sayables) => sayables.flatMap(_.toSingles)
+    case Sayable.Group(sayable) => sayable.toSingles
   }
   override def toString =
     toSingles
@@ -30,10 +31,17 @@ sealed trait Sayable {
 }
 object Sayable {
   sealed trait Single extends Sayable
+  case class Group(sayable: Sayable) extends Sayable
   case class Many(sayables: Seq[Sayable]) extends Sayable
 
   val Empty: Sayable = Many(Nil)
 
+  def apply(sayables: Seq[Sayable]) = sayables match {
+    case Seq()       => Empty
+    case Seq(single) => single
+    case many        => Many(many)
+  }
+
   def unapplySeq(sayable: Sayable): Option[Seq[Any]] = Some(sayable.toSingles.map {
     case speak: Speaks#Speak => speak.msg
     case s                   => s
@@ -44,12 +52,14 @@ object Sayable {
       case Pause(ms)       => pause(ms)
       case Play(path)      => play(path)
       case s: Speaks#Speak => speak(s)
+      case Group(s)        => group(s)
       case Many(sayables)  => many(sayables.toList)
     }
 
     def pause(ms: Int): A
     def play(path: AudioPath): A
     def speak(spk: Speaks#Speak): A
+    def group(sayable: Sayable): A
     def many(sayables: List[Sayable]): A
   }
 }

core/src/test/scala/simpleivr/ChoiceMenuTests.scala

@@ -0,0 +1,23 @@
+package simpleivr
+
+import org.scalatest.{FunSuite, Matchers}
+
+
+class ChoiceMenuTests extends FunSuite with Matchers {
+  test("assigned") {
+    def defined(key: DTMF) = Choice(key, Sayable.Empty, ())
+
+    def auto = Choice(Sayable.Empty, ())
+
+    val choices = List(defined(DTMF.`1`), auto, defined(DTMF.`2`), auto, auto, auto, auto, auto, auto, auto)
+
+    val choiceMenu = ChoiceMenu(Sayable.Empty, choices)
+
+    val actual = choiceMenu.assigned.map(_.dtmf)
+
+    val expected =
+      List(DTMF.`1`, DTMF.`3`, DTMF.`2`, DTMF.`4`, DTMF.`5`, DTMF.`6`, DTMF.`7`, DTMF.`8`, DTMF.`9`, DTMF.`0`)
+
+    actual shouldBe expected
+  }
+}

core/src/test/scala/simpleivr/DummySayables.scala

@@ -3,7 +3,9 @@ package simpleivr
 import java.nio.file.Files
 
 
-object DummySayables extends Sayables({
+class DummySayables extends Sayables({
   val dir = Files.createTempDirectory("simpleivr_DummySayables")
   new LocalAudioFileBackend(dir, dir.toString)
 })
+
+object DummySayables extends DummySayables

testing/src/main/scala/simpleivr/testing/InteractionIvrCommandInterpreter.scala

@@ -1,10 +1,10 @@
 package simpleivr.testing
 
-import simpleivr.{DTMF, Pause, Sayable}
+import simpleivr.{DTMF, Sayable}
 
 
 class InteractionIvrCommandInterpreter(var interactions: List[Interaction], override val callerId: String = null)
-                                      (checkIgnored: Int => Unit) extends NullIvrCommandInterpreter {
+                                      (checkIgnored: Int => Unit) extends NullIvrCommandInterpreter with ParseChoices {
   private[this] var ignoredCount = 0
 
   private def modHead[A](pf: PartialFunction[Interaction, (A, Option[Interaction])]): Option[A] = {
@@ -40,36 +40,11 @@ class InteractionIvrCommandInterpreter(var interactions: List[Interaction], over
     case Interaction.Press(Nil)                     => None -> None
   }
 
-  private object digit {
-    def unapply(sayable: Sayable): Option[DTMF] = sayable match {
-      case Sayable("one")   => Some(DTMF.`1`)
-      case Sayable("two")   => Some(DTMF.`2`)
-      case Sayable("three") => Some(DTMF.`3`)
-      case Sayable("four")  => Some(DTMF.`4`)
-      case Sayable("five")  => Some(DTMF.`5`)
-      case Sayable("six")   => Some(DTMF.`6`)
-      case Sayable("seven") => Some(DTMF.`7`)
-      case Sayable("eight") => Some(DTMF.`8`)
-      case Sayable("nine")  => Some(DTMF.`9`)
-      case Sayable("zero")  => Some(DTMF.`0`)
-      case Sayable("pound") => Some(DTMF.`#`)
-      case Sayable("star")  => Some(DTMF.*)
-    }
-  }
-
-  private def toChoices(sayables: List[Sayable.Single]): List[(DTMF, Sayable)] =
-    Util.spans(sayables) { case Pause(_) => true case _ => false }
-      .flatMap {
-        case Sayable("Press") +: digit(d) +: label => Some(d -> Sayable.Many(label))
-        case label :+ Sayable("Press") :+ digit(d) => Some(d -> Sayable.Many(label))
-        case other                                 => None
-      }
-
   override def waitForDigit(timeout: Int): Option[DTMF] = modHead(takeInput()).flatten
   override def say(sayable: Sayable, interruptDtmfs: Set[DTMF]): Option[DTMF] = {
     object choice {
       def unapply(text: String): Option[DTMF] =
-        toChoices(sayable.toSingles.toList)
+        parseChoices(sayable)
           .find(_._2.toString().contains(text))
           .map(_._1)
     }
@@ -86,5 +61,3 @@ class InteractionIvrCommandInterpreter(var interactions: List[Interaction], over
     0
   }
 }
-
-

testing/src/main/scala/simpleivr/testing/ParseChoices.scala

@@ -0,0 +1,33 @@
+package simpleivr.testing
+
+import simpleivr.Sayable.{Group, Many, Single}
+import simpleivr.{DTMF, Pause, Sayable}
+
+
+trait ParseChoices {
+  private object digit {
+    def unapply(sayable: Sayable): Option[DTMF] = sayable match {
+      case Sayable("one")   => Some(DTMF.`1`)
+      case Sayable("two")   => Some(DTMF.`2`)
+      case Sayable("three") => Some(DTMF.`3`)
+      case Sayable("four")  => Some(DTMF.`4`)
+      case Sayable("five")  => Some(DTMF.`5`)
+      case Sayable("six")   => Some(DTMF.`6`)
+      case Sayable("seven") => Some(DTMF.`7`)
+      case Sayable("eight") => Some(DTMF.`8`)
+      case Sayable("nine")  => Some(DTMF.`9`)
+      case Sayable("zero")  => Some(DTMF.`0`)
+      case Sayable("pound") => Some(DTMF.`#`)
+      case Sayable("star")  => Some(DTMF.*)
+    }
+  }
+
+  def parseChoices(sayable: Sayable): List[(DTMF, Sayable)] =
+    sayable match {
+      case s: Single                                               => Nil
+      case Many(Pause(_) +: Sayable("Press") +: digit(d) +: label) => List(d -> Sayable(label))
+      case Many(Pause(_) +: label :+ Sayable("Press") :+ digit(d)) => List(d -> Sayable(label))
+      case Many(sayables)                                          => sayables.flatMap(parseChoices).toList
+      case Group(s)                                                => parseChoices(s)
+    }
+}

testing/src/test/scala/simpleivr/testing/ParseChoiceTests.scala

@@ -0,0 +1,33 @@
+package simpleivr.testing
+
+import org.scalatest.FunSuite
+import simpleivr._
+
+
+class ParseChoiceTests extends FunSuite with ParseChoices {
+  test("parseChoices") {
+    object S extends DummySayables {
+      val `do one thing`, `do something else` = speak
+    }
+
+    val ivrChoices = new IvrChoices(S)
+    import ivrChoices._
+
+    val choices =
+      List(
+        Choice.Assigned(DTMF.`1`, S.`do one thing`, ()),
+        Choice.Assigned(DTMF.`2`, S.`do something else`, ())
+      )
+
+    val choiceMenu = ChoiceMenu(Sayable.Empty, choices)
+    val pauseMs = 750
+    val labelFirstParsed = parseChoices(choiceMenuSayable(choiceMenu, pauseMs, SayChoice.LabelFirst))
+    println(">>")
+    val labelLastParsed = parseChoices(choiceMenuSayable(choiceMenu, pauseMs, SayChoice.LabelLast))
+    println("<<")
+
+    assert(labelFirstParsed == labelLastParsed)
+
+    assert(labelFirstParsed == choices.map(c => c.dtmf -> c.label))
+  }
+}