0

I've refactored my code for day 12 of advent of code by using monocle, a lens library in scala.

Is it possible to improve this code :

  type Register = String
  type Mem = Map[String, Int]

  @Lenses
  case class State(mem: Mem, pointer: Int)

  def processInstruction(instructions: Seq[Instruction]): State => State = { s =>
    (instructions(s.pointer) match {
      case Inc(r) =>
        State.pointer.modify( _ + 1) andThen (State.mem composeLens at(r)).modify(_.map(_ + 1))
      case Dec(r) =>
        State.pointer.modify( _ + 1) andThen (State.mem composeLens at(r)).modify(_.map(_ - 1))
      case CpyInt(v, to) =>
        State.pointer.modify( _ + 1) andThen (State.mem composeLens at(to)).set(Some(v))
      case CpyReg(from, to) =>
        State.pointer.modify( _ + 1) andThen (State.mem composeLens at(to)).set(Some(s.mem(from)))
      case Jnz(r, v) => if (r != "1" && s.mem(r) == 0)
        State.pointer.modify( _ + 1)
      else
        State.pointer.modify( _ + v )
    }).apply(s)
  }

And here is another try, separating the modification of each field

  def processInstruction2(instructions: Seq[Instruction]): State => State = { s =>
    val ptr = instructions(s.pointer) match {
      case Jnz(r, v) if !(r != "1" && s.mem(r) == 0) => State.pointer.modify(_ + v)
      case _ => State.pointer.modify(_ + 1)
    }

    val mem = instructions(s.pointer) match {
    case Inc(r) => (State.mem composeLens at(r)).modify(_.map(_ + 1))
    case Dec(r) => (State.mem composeLens at(r)).modify(_.map(_ - 1))
    case CpyInt(v, to) => (State.mem composeLens at(to)).set(Some(v))
    case CpyReg(from, to) => (State.mem composeLens at(to)).set(Some(s.mem(from)))
    case _ => identity[State]
  }
    (ptr andThen mem)(s)
  }

One more question : is there a way to use Map.withDefaultValue with monocle ?

The full code is here : https://gist.github.com/YannMoisan/b8ba25afc041d88706545527d9ec1988

Endzeit
  • 4,810
  • 5
  • 29
  • 52
Yann Moisan
  • 8,161
  • 8
  • 47
  • 91
  • 1
    I would use `index` instead of `at` when you want to modify the value inside of the map, e.g. `(State.mem composeOptional index(r)).modify(_ + 1)` instead of `(State.mem composeLens at(r)).modify(_.map(_ + 1))` – Julien Truffaut Jan 11 '17 at 23:28

1 Answers1

-1

You might want to use the second approach, because it separates handling of two fields. However, the functions shouldn't be interpreted in sequence (andThen), but rather they should be combined as PartialFunctions with orElse.

def processInstruction3(instructions: Seq[Instruction]): State => State = {
  val ptr: PartialFunction[Instruction, State => State] = {
    case Jnz(r, v) =>
      State.pointer.modify(_ + v)
  }

  val incPointer: State => State = State.pointer.modify( _ + 1)
  def reg(r: String): Lens[State, Option[Int]] = State.mem composeLens at(r)
  val mem: PartialFunction[Instruction, State => State] = {
    case Inc(r) => reg(r).modify(_.orElse(Option(0)).map(_ + 1))
    case Dec(r) => reg(r).modify(_.orElse(Option(0)).map(_ - 1))
    case CpyInt(v, to) => reg(to).set(Some(v))
    case CpyReg(from, to) => s => reg(to).set(reg(from).get(s))(s)
  }
  val interpreter = ptr orElse (mem andThen (_ andThen incPointer))
  s => instructions.foldLeft(s)((s, i) => interpreter(i)(s))
}

UPDATE (after the Yann Moisan comment)

The execution may not terminate in case of infinite loop in user's program. So instead of the foldLeft we need some recursive function that will extract the next instruction by pointer:

@tailrec
def loop(s: State): State = {
  if(s.pointer>=instructions.length)
    s
  else {
    val instruction = instructions(s.pointer)
    val nextState = interpreter(instruction)(s)
    loop(nextState)
  }
}
loop _

(The last line of processInstruction3 should be replaced with the above code)

Arseniy Zhizhelev
  • 2,381
  • 17
  • 21
  • instructions should not be executed in order because of the jump (jnz) instruction. – Yann Moisan Jan 04 '17 at 21:03
  • Here are two aspects. One is modularity - handling of instructions of various kinds separately. And this is achieved by `PartialFunction`. The other aspect is the sequential run of two steps for normal instructions (execution and pointer advance). Both aspects are handled properly in `processInstruction3`. In the `processInstruction2` `(ptr andThen mem)` in case of `Jnz` it will be also handled by the second pattern matching block (luckily there is no other similar instruction and it'll go through default case). – Arseniy Zhizhelev Jan 05 '17 at 08:24