From 99b060854b539436673e5d4e400be3873b3736f6 Mon Sep 17 00:00:00 2001 From: Danny Navarro Date: Thu, 22 Sep 2016 16:40:10 -0300 Subject: [PATCH] Stylistic/Optimization changes Among them: - case deconstruction instead of assignment deconstruction. - `>>=` -> `flatMap`. - Remove `this`. - Use `collect` instead of list monad with for-comprehension. - More commenting. This takes care of comments for #85. --- .../edu/gemini/seqexec/engine/model.scala | 41 ++++++++++--------- .../edu/gemini/seqexec/engine/package.scala | 29 +++++++------ 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/model.scala b/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/model.scala index 706d86ab23..c264ff0d51 100644 --- a/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/model.scala +++ b/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/model.scala @@ -12,7 +12,7 @@ case class QState(pending: Queue[Action], done: Queue[Result], status: Status) { - def isEmpty: Boolean = this.pending.sequences.isEmpty && this.current.isEmpty + def isEmpty: Boolean = pending.sequences.isEmpty && current.isEmpty } object QState { @@ -58,18 +58,15 @@ object QState { * If the `Current` doesn't have all actions completed or there are no more * pending `Execution`s it returns None. */ - def next(qs: QState): Option[QState] = cleanup(qs) >>= (prime(_)) + def next(qs: QState): Option[QState] = cleanup(qs).flatMap(prime) // None: current not empty // pending queue empty def prime(qs: QState): Option[QState] = if (qs.current.isEmpty) - Queue.uncons(qs.pending).map( - t => { - val (exe3, q) = t - QState(q, currentify(exe3), qs.done, qs.status) - } - ) + Queue.uncons(qs.pending).map { + case (exe3, q) => QState(q, currentify(exe3), qs.done, qs.status) + } else None def cleanup(qs: QState): Option[QState] = @@ -81,13 +78,16 @@ object QState { * returning the unwrapped `Execution`. */ // TODO: Use same structure for `Current` and `Queue.Execution3`? - private def currentify(exe3: Queue.Execution3[Action]): (Current) = { + private def currentify(exe3: Queue.Execution3[Action]): Current = { def vec(exe: Execution[Action]): Vector[Action \/ Result] = exe.map(_.left).toVector exe3 match { + // New Sequence case -\/(-\/((actions, seqid, stepid))) => Current(vec(actions), Some((seqid, stepid).left)) + // New Step case -\/(\/-((actions, stepid))) => Current(vec(actions), Some(stepid.right)) + // Modify current Step case \/-(actions) => Current(vec(actions), None) } } @@ -105,8 +105,11 @@ object QState { unvec.map( exe => current.ctxt match { + // Modify current Step case None => exe.right + // New Sequence case Some(-\/((seqid, stepid))) => (exe, seqid, stepid).left.left + // New Step case Some(\/-(stepid)) => (exe, stepid).right.left } ) @@ -128,14 +131,14 @@ object Status { * for the addition of new ones. */ case class Queue[A](sequences: List[Sequence[A]]) { - def isEmpty: Boolean = this.sequences.isEmpty + def isEmpty: Boolean = sequences.isEmpty } object Queue { type Execution3[A] = (Execution[A], String, Int) \/ (Execution[A], Int) \/ Execution[A] - def empty[A]: Queue[A] = Queue(List()) + def empty[A]: Queue[A] = Queue(Nil) def sequences[A]: Queue[A] @> List[Sequence[A]] = Lens.lensu((q, ss) => q.copy(sequences = ss), _.sequences) @@ -202,7 +205,7 @@ object Queue { // No more Steps in current Sequence, remove Sequence. // TODO: listTailPLens? case None => ((exe, seq0.id, stepid).left.left, - Queue(q.sequences.tailOption.getOrElse(List()))) + Queue(q.sequences.tailOption.getOrElse(Nil))) // More Steps left in current Sequence, remove `Step` from Sequence. case Some(seq) => ((exe, stepid).right.left, head.set(q, seq).getOrElse(q)) } @@ -272,7 +275,7 @@ object Sequence { } /** - * A list of `Executions` grouped by obersvation. + * A list of `Executions` grouped by observation. */ case class Step[A](id: Int, executions: NonEmptyList[Execution[A]]) @@ -309,21 +312,19 @@ case class Current(ars: Vector[Action \/ Result], // Sequence or Step parameters ctxt: Option[(String, Int) \/ Int]) { - def isEmpty: Boolean = this.ars.empty + def isEmpty: Boolean = ars.empty def actions: List[Action] = { - // not available in scalaz? - def lefts[L, R](xs: List[L \/ R]): List[L] = for { -\/(r) <- xs } yield r + def lefts[L, R](xs: List[L \/ R]): List[L] = xs.collect { case -\/(l) => l } - lefts(this.ars.toList) + lefts(ars.toList) } def results: List[Result] = { - // not available in scalaz? - def rights[L, R](xs: List[L \/ R]): List[R] = for { \/-(r) <- xs } yield r + def rights[L, R](xs: List[L \/ R]): List[R] = xs.collect { case \/-(r) => r } - rights(this.ars.toList) + rights(ars.toList) } } diff --git a/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/package.scala b/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/package.scala index 8d30534045..df4988881f 100644 --- a/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/package.scala +++ b/modules/edu.gemini.seqexec.engine/src/main/scala/edu/gemini/seqexec/engine/package.scala @@ -44,13 +44,12 @@ package object engine { * `Running` status. */ def switch(q: EventQueue)(st: Status): Engine[QState] = - modify(QState.status.set(_, st)) *> { - if (st == Status.Running) prime *> execute(q) *> get - else get - } + modify(QState.status.set(_, st)) *> + whenM (st == Status.Running) (prime *> execute(q)) *> + get def prime: Engine[Unit] = - gets(QState.prime(_)) >>= { + gets(QState.prime(_)).flatMap { case None => unit case Some(qs) => put(qs) } @@ -62,7 +61,7 @@ package object engine { * If there are no more pending `Execution`s, it emits the `Finished` event. */ def next(q: EventQueue): Engine[QState] = - (gets(QState.next(_)) >>= { + (gets(QState.next(_)).flatMap { // No more Executions left case None => send(q)(finished) // Execution completed, execute next actions @@ -77,20 +76,20 @@ package object engine { private def execute(q: EventQueue): Engine[Unit] = { // Send the expected event when action is executed - def act(t: (Action, Int)): Task[Unit] = { - val (action, i) = t - action >>= { - case Result.OK => q.enqueueOne(completed(i)) - case Result.Error => q.enqueueOne(failed(i)) - } + def act(t: (Action, Int)): Task[Unit] = t match { + case (action, i) => + action.flatMap { + case Result.OK => q.enqueueOne(completed(i)) + case Result.Error => q.enqueueOne(failed(i)) + } } - status >>= { + status.flatMap { case Status.Waiting => unit case Status.Running => ( - gets(_.current.actions) >>= ( + gets(_.current.actions).flatMap( actions => Nondeterminism[Task].gatherUnordered( - actions.zipWithIndex.map(act(_)) + actions.zipWithIndex.map(act) ).liftM[EngineStateT] ) ) *> send(q)(executed)