Hi, some opinionated thoughts.
Personally, I think that modularising some code is mostly about four things, Naming, Categorisation, Reuse, Encapsulation.
Naming
The smaller a module is, the less chance there is for names to start bumping up against one another. For example in the elm/core library there are several modules and many define a map function: List.map, Dict.map, Platform.Cmd.map, etc. We could just have all of elm/core in one large module, but then we would have to call these functions listMap, dictMap etc. Itâs arguable which one is better, however, the former kind promotes consistency, whereas the latter kind itâs very easy to end up with listMap and mapDict.
As a side note, I generally encourage âqualified importâ. So for example you have imported a lot of names unqualified, e.g. import String exposing (fromFloat, fromInt, toInt) this means two things. First of all if you have a module of your own that can take some data type to/from Ints/Floats then you either have to break your convention and qualify one of the imports or name your function something like yearFromFloat. If you instead have the general (breakable) convention that you qualify your imports then you just import String and in your code you refer to String.fromInt. To my mind this is better because the name you are using is more descriptive of what is happening. Additionally you donât have to edit your imports as often. However, there are certainly other opinions available.
Categorisation
This is a dubious benefit, but if I see a name such as Year.fromFloat I know where to look for it, it must be in a Year module. Of course, you should have some editor/IDE support for being able to go directly to the definition so this is relatively dubious. Nonetheless I think a general sense of âorderlynessâ/âtidynessâ is behind a lot of modularisation (and probably the cause of most premature modularisation).
Reuse
Of course a function is re-usable as well. But if youâre going to use a type plus some related function definitions, in more than one place in your project then a module can communicate the intent of that relatively well.
In Elm, I find this to be a bit less important than in other projects. A common boundary, is between âlogicâ and user-interface. Such that you could use the same logic for multiple different user-interfaces. Because if youâre using Elm, youâre (mostly) committing to a Web user-interface, so youâre unlikely to use the same logic for, say, a TUI interface. However, you might, in the same project, define some kind of entity with related logic, and then two separate views of that entity, say as part of a list or as a more detailed page all to itself (e.g. blog posts viewed as a list in the home page, but then as a single page for each one).
Encapsulation
I wonât go into this one in detail, but the idea is to restrict the visibility of an underlying data type so that itâs impossible from outwith the defining module to create an invalid value of the given type.
For your project
Iâd say you should start by separating the logic from the viewing code. Youâve so far done a pretty reasonable job of this, however you have a bit of a âcode smellâ in that you have âmagic numbersâ in the view code. Here is an example from viewDeclination:
, p [] [ text (" Day Length = " ++ mnToHrMn (60 * getDayLength inputdata)) ]
How does 60 relate to the âday lengthâ? The function getDayLength itself is a little non-descriptive, get day length in whats? seconds? milliseconds? human reaction times? In this case itâs a little worse because 60 is the number of seconds in a minute and also the number of minutes in an hour, so Iâm not sure what this 60 represents. Much better if you have a module SunData (or InputData or whatever), with a function getMinutesInDay this means your code now looks like:
, p [] [ text (" Day Length = " ++ mnToHrMn (SunData.getMinutesInDay inputData )) ]
Data type in Main/WorkSunCalculator
WorkSunCalculator needs this InputData. So either you define it there, or in a separate module (I would vote for there, donât do unnecessary modularisation, and when you have a module that contains only a type definition there is a good chance youâre doing something prematurely/unnecessarily).
However, you currently define the exact same type in Main and WorkSunCalculator. You donât need this, just refer to WorkSunCalculator.InputData in your Main module. If you like you can do the following in Main:
type alias Model = WorkSunCalculator.InputData
This means you can later change the type of Model if you need to store additional information. For example, you might store the current time (perhaps updated by a subscription Time.every).
type alias Model =
{ now : Time.Posix
, inputData : WorkSunCalculator.InputData
}
Now, you donât have to change the WorkSunCalculator module at all. The changes to Main are fairly trivial, wherever you make a call to a function in WorkSunCalculator that accepts an InputData you just have to update the argument e.g. WorkSunCalculator.getMinutesInDay model is now WorkSunCalculator.getMinutesInDay model.inputData. (Note, by keeping WorkSunCalculator module qualified import the places you need to change are easily identifiable, though of course the compiler would identify them for you anyway).
Final unrelated point
You have several places in your view code, where you seem to be intending that a list of things all look the same, but youâre doing so implicitly rather than explicitly, letâs look at your viewDeclination function again:
viewDeclination : InputData -> Html msg
viewDeclination inputdata =
div [ style "color" "green", style "font-size" "1.4em"]
[ p [] [ text (" Sun Declination = " ++ cutDec6 (sunDeclination inputdata) ++ "°") ]
, p [] [ text (" Day Length = " ++ mnToHrMn (60 * getDayLength inputdata)) ]
, p [] [ text (" Civil Twilight = " ++ mnToHrMn (civTwlMns inputdata -1) ++ locTZ inputdata) ]
, p [] [ text (morningToNoon inputdata) ]
... bunch of similar others
It seems like you want all of these elements to look the same, but itâs unclear from this if this is by accident or design. You can make it explicitly by design:
viewDeclination : InputData -> Html msg
viewDeclination inputdata =
let
viewDeclinationItem : String -> Html msg
viewDeclinationItem content =
p [] [ text content]
in
div [ style "color" "green", style "font-size" "1.4em"]
[ viewDeclationItem (" Sun Declination = " ++ cutDec6 (sunDeclination inputdata) ++ "°")
, viewDeclationItem (" Day Length = " ++ mnToHrMn (60 * getDayLength inputdata))
, viewDeclationItem (" Civil Twilight = " ++ mnToHrMn (civTwlMns inputdata -1) ++ locTZ inputdata)
, viewDeclationItem (morningToNoon inputdata)
... bunch of similar others
Or even
viewDeclination : InputData -> Html msg
viewDeclination inputdata =
let
viewDeclinationItem : String -> Html msg
viewDeclinationItem content =
p [] [ text content]
items : List String
items =
[ " Sun Declination = " ++ cutDec6 (sunDeclination inputdata) ++ "°"
, " Day Length = " ++ mnToHrMn (60 * getDayLength inputdata)
, " Civil Twilight = " ++ mnToHrMn (civTwlMns inputdata -1) ++ locTZ inputdata
, morningToNoon inputdata
... bunch of similar others
]
in
div [ style "color" "green", style "font-size" "1.4em"]
(List.map viewDeclinationItem items)
Depends on whether you are likely to have non-declination items within that list or not.
Even in other places where itâs not quite the same, sometimes factoring out the common parts make the differences stand out:
viewValidation : InputData -> Html msg
viewValidation inputData =
if {- condition -}
then
div [ style "color" "blue" ] [ text "Date entry OK" ]
else
div [ style "color" "red" ] [ text "Incorrect month or day" ]
Note how here these are the same other than the color and the actual text, but it takes some scanning and checking to work that out:
viewValidation : InputData -> Html msg
viewValidation inputData =
let
viewValidationDiv : String -> String -> Html msg
viewValidationDiv color content =
div [ style "color" color ] [ text content ]
in
if {- condition -}
then
viewValidationDiv "blue" "Data entry OK"
else
viewValidationDiv "red" "Incorrect month or day"
Now the differences between the two cases couldnât be any clearer.
