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 ControlsMsgs:
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.