Random Dice Simulation

Question

I extended the example in 04-random.elm to support N dice and to draw the results using SVG.

import Html exposing (..)
import Html.Events exposing (..)
import Random
import List exposing (length, repeat, range)
import Svg exposing (Svg, svg, circle)
import Svg.Attributes exposing (..)
main =
 Html.program {
 init = init,
 view = view,
 update = update,
 subscriptions = subscriptions
 }
-- MODEL
type alias Model = {
 dieFaces : List Int
}
init : (Model, Cmd Msg)
init =
 let numDice = 5
 in
 (Model (repeat numDice 1), Cmd.none)
-- UPDATE
type Msg =
 Roll |
 NewFaces (List Int)
update : Msg -> Model -> (Model, Cmd Msg)
update msg model =
 case msg of
 Roll -> (
 model,
 Random.generate NewFaces (
 Random.list (length model.dieFaces) (Random.int 1 6)
 )
 )
 NewFaces newFaces ->
 (Model newFaces, Cmd.none)
-- SUBSCRIPTIONS
subscriptions : Model -> Sub Msg
subscriptions model =
 Sub.none
-- VIEW
view : Model -> Html Msg
view model =
 div [] (
 List.map drawFace model.dieFaces
 ++ [ button [ onClick Roll ] [ text "Roll" ] ]
 )
drawFace : Int -> Html Msg
drawFace numDots =
 div [ style "width: 66px; height: 66px; padding: 10px" ] [
 svg
 [ version "1.1", width "66", height "66" ]
 (drawDice ::
 (List.map (\i -> drawDot i numDots) (range 0 (numDots - 1))))
 ]
drawDice: Svg Msg
drawDice =
 circle [ fill "gold", stroke "orange", strokeWidth "3", cx "33", cy "33", r "30" ] []
drawDot: Int -> Int -> Svg Msg
drawDot i n =
 circle [ fill "black", cx (toString (posX i n)), cy (toString (posY i n)), r "5" ] []
posX: Int -> Int -> Int
posX i n =
 round (33 + (15 * cos (6.28 * (toFloat(i) / toFloat(n)))))
posY: Int -> Int -> Int
posY i n =
 round (33 + (15 * sin (6.28 * (toFloat(i) / toFloat(n)))))

Answer 1

This review comes with the caveat that I've spent a few days in total on Elm so far... Take anything I say with a pinch of salt.

Firstly, some style things.

posX: Int -> Int -> Int
posX i n =
 round (33 + (15 * cos (6.28 * (toFloat(i) / toFloat(n)))))

Why so many brackets? You can cut it down to this very easily

posX i n =
 round (33 + 15 * cos (6.28 * toFloat(i) / toFloat(n)))

But you can make it even clearer by using a |>

posX i n = 
 33 + 15 * cos (6.28 * toFloat(i) / toFloat(n))
 |> round

I like this more because it also promotes the calculation to its own line where nothing else happens. You could also have written:

posX i n = 
 round <| 33 + 15 * cos (6.28 * toFloat(i) / toFloat(n))

I think it's up to you which one you prefer. I like to think about getting the value first and then rounding it.

There is a similar thing here:

drawFace : Int -> Html Msg
drawFace numDots =
 div [ style "width: 66px; height: 66px; padding: 10px" ] [
 svg
 [ version "1.1", width "66", height "66" ]
 (drawDice ::
 (List.map (\i -> drawDot i numDots) (range 0 (numDots - 1))))
 ]

I would instead have:

drawFace : Int -> Html Msg
drawFace numDots =
 div [ style "width: 66px; height: 66px; padding: 10px" ] [
 svg
 [ version "1.1", width "66", height "66" ]
 (range 0 (numDots - 1)
 |> List.map (\i -> drawDot i numDots) 
 |> (::) drawDice
 )
 ]

It may look more complex but it's easier to follow in my opinion because each line does one thing.

1. Create your range
2. Map each to an Svg Msg (your dots)
3. Cons the face to the list of dots

Now, if you change the order of your arguments to drawDot you can simplify it further with currying.

drawDot: Int -> Int -> Svg Msg
drawDot n i =
 circle [ fill "black", cx (toString (posX i n)), cy (toString (posY i n)), r "5" ] []
(range 0 (numDots - 1)
 |> List.map (drawDot numDots) 
 |> (::) drawDice
)

You'll find that you need to think about the usage of the function before writing it so you can get the parameter order right for currying.

In my limited understanding/knowledge of Elm, I'd say that your code is pretty good :)