Very cool animation! A few thoughts as I go through the code.
–
I see that you import things like Maybe, List, etc.
Some of these things (the ones listed in https://package.elm-lang.org/packages/elm/core/latest/) are imported by default, so you don’t need to do it again.
–
Another thing that strikes me is that you have potentially conflicting sources of truth with the neighbours in the Cell dictionary, and the list of white and black elements positions.
Your model could look potentially end up looking like this (if you have a bug in your logic somewhere):
{ board: <empty dict>
, l : [(1, 1)]
, w : []
, ...
}
-- Or the other way around
{ board: Dict.fromList [(1, 1), {state: Alive, color: White, neighbours: 5 }]
, l : []
, w : []
, ...
}
In the first model, you have an empty board but you somehow have a white cell. In the second model, you have a non-empty board, but no white or black cells.
In both cases, something is off, and ideally, we would like that to not happen.
I suggest (re)watching this talk on “Making Impossible States Impossible” https://www.youtube.com/watch?v=IcgmSRJHu_8
I think you compute the neighbours to avoid doing it again later (I might be wrong, I haven’t delved in this enough), but It’s probably better if you only have board without the neighbours, or only l and w, or another similar data structure containing the information you really want.
You would then have to compute the neighbours on every update that affects the board, but you kind of already do that. This way you would only have a single source of truth, which should lead to less bugs (you can create a dictionary of neighbours, just don’t store it in the model), but also probably remove quite a lot of functions related to assigning neighbours.
–
In the aliveClass, I see that you add a default pattern for the alive case. This is fine at the moment, but if one day you change the type for Cell (add a new value for instance), then potentially this could lead to a bug. I would suggest replacing _ -> "alive" by Alive -> "alive", that way when you change Cell, the compiler will have your back.
The same comment applies to functions like isSpawning. It may be a bit more lengthy to write all the cases, but I generally try to specify every matching pattern instead of using _ ->, so that the compiler can help me out. Maybe I’ll need to write NewCellConstructorName -> False every time, but at least the decision will be conscious and I won’t have to think “Oh, was there be a location where I should have changed how the pattern match was done?”.
–
I agree with what @folkertdev said, "I almost always prefer parens over <|". I usually prefer using |> over parens or <| (personal preference, do what you want with it).
onlyAlive =
List.foldl
spawnToAlive
clearedDead
(List.map
(\( idx, _ ) -> idx)
(Dict.toList
(Dict.filter (\idx c -> isSpawning c) clearedDead)
)
)
-- Becomes
onlyAlive =
clearedDead
|> Dict.filter (\idx c -> isSpawning c)
|> Dict.toList
|> List.map (\( idx, _ ) -> idx)
|> List.foldl spawnToAlive clearedDead
-- Which I notice can probably be simplified to
onlyAlive =
clearedDead
|> Dict.filter (\idx c -> isSpawning c)
|> Dict.keys
|> List.foldl spawnToAlive clearedDead
Happy learning!
