HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Solving adventcode day 6 puzzle: toggling lights in a grid

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
solvinggridtogglinglightspuzzledayadventcode

Problem

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 mean

Solution

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]  followInstructions English
|> Seq.cast
|> 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
|> 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 nu

Code Snippets

let toggle = function
            | Switch.On -> Switch.Off
            | _ -> Switch.On
let toggle = function
            | Switch.On -> Switch.Off
            | _         -> Switch.On
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 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 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"

Context

StackExchange Code Review Q#115600, answer score: 3

Revisions (0)

No revisions yet.