I’d say you’ve done a pretty good job here. I wouldn’t change much if anything.
A couple of minor personal things:
- In general, not just in this repo, I see quite a lot of use of
|>
where I feel it’s pretty unnecessary. To me |>
is useful when you have a bunch of operations to perform and you just want to write them down in the order that they are performed, rather than in function application order. So, I kind of don’t see the point of:
{ model | speed = nextSpeed model.speed }
|> withoutCmd
Especially when you consider the definition of withoutCmd
you could just have written this as:
({ model | speed = nextSpeed model.speed}
, Cmd.none
)
But I’m 100% sure others would prefer what you’ve written.
In Matix.elm you have a wrap
function:
wrap : Int -> Int -> Int -> Int
wrap min max value =
if value < min then
max
else if value > max then
min
else
value
Just to say that min
and max
are defined in Basics
:
wrap : Int -> Int -> Int -> Int
wrap smallest largest value =
max smallest <| min largest value
Finally something more what you were asking. I personally find your module structure fine. It’s probably a little “over-modularised” given that this program is unlikely to grow significantly. You’re pretty much done. But assuming you were meaning this as a kind of exercise it’s fine.
Your main pain point is the Controls
module. It is kind of conceivable that you could re-use this for a different application, but fairly unlikely, and doing so might make it awkward to add a new control specific to this game. In either case the pain point is that you’re insistent that the controls are polymorphic in the type of the message, meaning that you have to pass in a record structure with all that information. But why?
You can get rid of that whole record definition by defining a type within the Controls
module:
type ControlsMsg
= StepBack
| StepForward
| etc.
Then in your Main
module you can define your Msg
type with one constructor for a ControlsMsg
:
type Msg
= ControlsMsg ControlsMsg
| ClockTick
| etc.
Then in your viewControls
function you just call Html.map ControlsMsg
on the Html
returned from Controls.view
.
That also means your update
function can look a little nicer as well as you can have a separate function to deal with all the ControlsMsg
s:
update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
case msg of
ControlsMsg controlsMsg ->
updateControls controlsMsg model
ClockTick ->
stepGame model
|> ifGameFinished pauseGame
|> withoutCmd
....
Or, you could just define your Msg
type in a module of its own, and just directly refer to it from both Controls.elm
and Main.elm
.