Solving adventcode day 6 puzzle: toggling lights in a grid

Question

I have solved the day 6 adventcode problem:

--- Day 6: Probably a Fire Hazard ---

Because your neighbors keep defeating you in the holiday house decorating contest year after year, you've decided to deploy one million lights in a 1000x1000 grid.

Furthermore, because you've been especially nice this year, Santa has mailed you instructions on how to display the ideal lighting configuration.

Lights in your grid are numbered from 0 to 999 in each direction; the lights at each corner are at 0,0, 0,999, 999,999, and 999,0. The instructions include whether to turn on, turn off, or toggle various inclusive ranges given as coordinate pairs. Each coordinate pair represents opposite corners of a rectangle, inclusive; a coordinate pair like 0,0 through 2,2 therefore refers to 9 lights in a 3x3 square. The lights all start turned off.

To defeat your neighbors this year, all you have to do is set up your lights by doing the instructions Santa sent you in order.

For example:

turn on 0,0 through 999,999 would turn on (or leave on) every light. toggle 0,0 through 999,0 would toggle the first line of 1000 lights, turning off the ones that were on, and turning on the ones that were off. turn off 499,499 through 500,500 would turn off (or leave off) the middle four lights. After following the instructions, how many lights are lit?

--- Part Two ---

You just finish implementing your winning light pattern when you realize you mistranslated Santa's message from Ancient Nordic Elvish.

The light grid you bought actually has individual brightness controls; each light can have a brightness of zero or more. The lights all start at zero.

The phrase turn on actually means that you should increase the brightness of those lights by 1.

The phrase turn off actually means that you should decrease the brightness of those lights by 1, to a minimum of zero.

The phrase toggle actually means that you should increase the brightness of those lights by 2.

What is the total brightness of all lights combined after following Santa's instructions?

For example:

turn on 0,0 through 0,0 would increase the total brightness by 1. toggle 0,0 through 999,999 would increase the total brightness by 2000000.

Here is my F# code. I would like feedback how to improve this.

https://github.com/naveensrinivasan/adventcode/blob/master/6.fsx

open System.IO
open System.Text.RegularExpressions

let filereadlines f = File.ReadAllLines(Path.Combine(__SOURCE_DIRECTORY__, f ))

type Switch = On | Off | Brightness of int

type Action = On | Off | Toggle

type MessageLanguage = Nordic | English

type Instruction = {
  Operation : Action ;
  StartRow  : int;
  StartCol : int;
  EndRow : int;
  EndCol : int;
  Language  : MessageLanguage
}

let toggle = function
            | Switch.On -> Switch.Off 
            |_  ->  Switch.On 

let getInstruction (line:string) (lang:MessageLanguage)= 
                  let matches = Regex.Matches(line, "[\w\d_]+")
                                        |> Seq.cast<Match>
                                        |> Seq.filter (fun f -> f.Success) |> Seq.map(fun f-> f.Value) 

                  let action = if matches |> Seq.nth 0 = "toggle" then Toggle
                                  elif matches |> Seq.nth 1 = "on"   then  On
                                  else Off 

                  let elementat n= matches |> Seq.nth n |> int

                  match action with
                  |Toggle -> {Operation = action; StartRow = elementat 1; StartCol = elementat 2; 
                                EndRow = elementat 4 ; EndCol = elementat 5; Language = lang}
                  |_ -> {Operation = action; StartRow = elementat 2; StartCol = elementat 3;
                          EndRow = elementat 5 ; EndCol = elementat 6; Language= lang} 

let translateCode (state:Switch) (lang :MessageLanguage) (action:Action) =
    match lang,action,state with
    |English, Toggle,_ ->  toggle state 
    |English, On,_ ->   Switch.On 
    |Nordic, Toggle,Brightness(x)  ->  Brightness (x+2) 
    |Nordic, On,Brightness(x) ->  Brightness (x+1) 
    |Nordic, Off ,Brightness(x) when x = 0 ->  Brightness (0) 
    |Nordic, Off ,Brightness(x)  ->  Brightness (x-1) 
    |_,_,_ ->  Switch.Off 

let lights =Array2D.create 1000 1000 Switch.Off

"6.txt"
|> filereadlines
|> Seq.map (fun f -> getInstruction f English)
|> Seq.iter(fun ins -> 
        for i in  ins.StartRow .. ins.EndRow do
            for j in ins.StartCol .. ins.EndCol  do
                lights.[i, j] <- translateCode lights.[i, j] ins.Language ins.Operation)

lights 
|> Seq.cast<Switch> 
|> Seq.filter(fun f -> f = Switch.On)
|> Seq.length
|> printfn "The number of lights that are turned on are %i"


let nordiclights =Array2D.create 1000 1000 (Brightness 0)
"6.txt"
|> filereadlines
|> Seq.map (fun f -> getInstruction f Nordic)
|> Seq.iter(fun ins -> 
        for i in  ins.StartRow .. ins.EndRow do
            for j in ins.StartCol .. ins.EndCol  do
                nordiclights.[i, j] <- translateCode nordiclights.[i, j] ins.Language ins.Operation)

nordiclights 
|> Seq.cast<Switch> 
|> Seq.map(fun f -> match f with
                       |Brightness(x) -> x
                       |_ -> 0)
|> Seq.sum 
|> printfn "The Santa's real nordic decoded message and the total brightness is %i"

Answer 1

First off: good job on the code! It's nice and clear, and easy to follow what it does and what it should do.

Style

Spaces

First off, a small remark about the spacing. I think it's interesting to write pattern matching without a space after the first character (|), because it looks like some ascii tree if you end the list with the "default" matcher (|_). But I would not recommend it, especially while the spacing is not really consistent in the file.

I saw you already reformatted the code on github, which is a good thing! Consistent spacing makes it easier for other programmers to read the code and not get distracted.

Alignment of pattern matching

Although this partly depends on what kind of editor you are using, you could make some pattern matchings more clear by aligning them. For instance:

let toggle = function
            | Switch.On -> Switch.Off
            | _ -> Switch.On

could become

let toggle = function
            | Switch.On -> Switch.Off
            | _         -> Switch.On

And

let translateCode (state: Switch) (lang: MessageLanguage) (action: Action) =
    match lang, action, state with
        | English, Toggle, _ -> toggle state
        | English, On, _ -> Switch.On
        | Nordic, Toggle, Brightness(x) -> Brightness (x+2)
        | Nordic, On, Brightness(x) -> Brightness (x+1)
        | Nordic, Off, Brightness(x) when x = 0 -> Brightness (0)
        | Nordic, Off, Brightness(x) -> Brightness (x-1)
        | _, _, _ -> Switch.Off

reads easier as

let translateCode (state: Switch) (lang: MessageLanguage) (action: Action) =
    match lang, action, state with
        | English, Toggle, _                        -> toggle state
        | English, On,     _                        -> Switch.On
        | Nordic,  Toggle, Brightness(x)            -> Brightness (x+2)
        | Nordic,  On,     Brightness(x)            -> Brightness (x+1)
        | Nordic,  Off,    Brightness(x) when x = 0 -> Brightness (0)
        | Nordic,  Off,    Brightness(x)            -> Brightness (x-1)
        | _,       _,      _                        -> Switch.Off

Again, this may or may not be easy to do with your editor (or that of others who will maintain this code), so it's up to you how to align these.

Duplicate code

You may have noticed, the code that solves each of the two problems is very similar. Actually, the only places where the two blocks of code differ, are the name of the array to adjust, and the name of the language to use for translation.

We can easily extract a function that can be used for both, if we give it those two changing elements as parameters:

let followInstructions (language: MessageLanguage) (lights: Switch[,]) =
    "6.txt"
    |> filereadlines
    |> Seq.map (fun f -> getInstruction f language)
    |> Seq.iter(fun ins ->
        for i in ins.StartRow .. ins.EndRow do
            for j in ins.StartCol .. ins.EndCol do
                lights.[i, j] <- translateCode lights.[i, j] ins.Language ins.Operation)
    lights

let lights = Array2D.create 1000 1000 Switch.Off
lights
|> followInstructions English
|> Seq.cast<Switch>
|> Seq.filter(fun f -> f = Switch.On)
|> Seq.length
|> printfn "The number of lights that are turned on are %i"

let nordiclights = Array2D.create 1000 1000 (Brightness 0)
nordiclights
|> followInstructions Nordic
|> Seq.cast<Switch>
|> Seq.map(fun f -> match f with
                        | Brightness(x) -> x
                        | _             -> 0)
|> Seq.sum
|> printfn "The Santa's real nordic decoded message and the total brightness is %i"

Don't put the MessageLanguge in the Instruction

The Instruction type, and therefor the getInstruction function, both keep track of the MessageLanguage. The instruction does not contain that information, and only when we translate the instruction we need that information. If you remove this field from the Instruction record, the code can be a bit easier to read:

type Instruction = {
  Operation: Action
  StartRow: int
  StartCol: int
  EndRow: int
  EndCol: int
}

let getInstruction (line: string) =
    // ...
    match action with
        | Toggle -> {Operation = action; StartRow = elementat 1; StartCol = elementat 2;
                     EndRow = elementat 4 ; EndCol = elementat 5}
        | _      -> {Operation = action; StartRow = elementat 2; StartCol = elementat 3;
                     EndRow = elementat 5 ; EndCol = elementat 6}

let followInstructions (language: MessageLanguage) (lights: Switch[,]) =
    // ...
    |> Seq.map (fun f -> getInstruction f)
    // ...
                    lights.[i, j] <- translateCode lights.[i, j] language ins.Operation)

In fact, that line with Seq.map (fun f -> getInstruction f) can now benefit from η-reduction ("eta reduction"), which means that you don't need to write a placeholder variable name:

|> Seq.map getInstruction

Use pattern matching when decoding

The getInstruction function can be written more transparent if you use pattern matching on the list of matches instead of if statements. This will also remove the need for numerous elementat calls:

let getInstruction (line: string) =
    let matches =
        Regex.Matches(line, "[\w\d_]+")
        |> Seq.cast<Match>
        |> Seq.filter (fun f -> f.Success) |> Seq.map(fun f-> f.Value)
        |> Seq.toList

    let operation, [startRow; startCol; _; endRow; endCol] =
        match matches with
            | "toggle" ::lights          -> Toggle, lights
            | "turn"   ::"on"   ::lights -> On, lights
            | "turn"   ::"off"  ::lights -> Off, lights
            | _                          -> failwith "Bad instruction format"

    { Operation = operation; StartRow = int startRow; StartCol = int startCol; EndRow = int endRow; EndCol = int endCol }

I also added a failWith there, instead of defaulting to Off on invalid input.

Parameter ordering and partial application

In F#, as in other functional languages, it's customary to order parameters from "least likely to vary" to "most likely to vary". This way, you can benefit from partial application, making calls a bit shorter. You can do this in translateCode, by moving lang to the front. The partial application will then look as follows:

let translateCode (lang: MessageLanguage) (state: Switch) (action: Action) =
    // ...

let followInstructions (language: MessageLanguage) (lights: Switch[,]) =
    let translate = translateCode language
    "6.txt"
    |> filereadlines
    |> Seq.map getInstruction
    |> Seq.iter(fun ins ->
        for i in ins.StartRow .. ins.EndRow do
            for j in ins.StartCol .. ins.EndCol do
                lights.[i, j] <- translate lights.[i, j] ins.Operation)
    lights

Now followInstructions creates a partial applied function translate, and no longer needs to pass the language every time when calling it.

Matching on literals

In translateCode, you can match on a literal 0 instead of using a when clause.

| Nordic,  Off,    Brightness(x) when x = 0 -> Brightness (0)

can be written as

| Nordic,  Off,    Brightness(0) -> Brightness (0)

Or you could combine the two matches on Nordic, Off, using the max function:

| Nordic,  Off,    Brightness(x) -> Brightness (max (x-1) 0)