evancz / elm-todomvc

The TodoMVC app written in Elm, nice example for beginners.
http://evancz.github.io/elm-todomvc/
BSD 3-Clause "New" or "Revised" License
1.21k stars 329 forks source link

Visibility #55

Open zwhitchcox opened 8 years ago

zwhitchcox commented 8 years ago

Is there a reason not to use a Union Type for the visibility?

https://github.com/evancz/elm-todomvc/blob/master/Todo.elm#L78

gurdiga commented 8 years ago

I was sketching out some Elm this morning and stumbled upon this nuance myself: strings are easy to mistype. 🤔

So yes: How to avoid hard-coding strings in conditionals like this:

    let
        isVisible todo =
            case visibility of
                "Completed" ->
                    todo.completed

                "Active" ->
                    not todo.completed

So here, if I say "Complted" instead of "Completed" the program just won’t work, and I’m wondering of there is an idiomatic way to approach this kind of issue. 🤓

gurdiga commented 8 years ago

I’ve tried (https://github.com/gurdiga/elm-todomvc/commit/8977837b0e35458323bfddc79fe63cac9f019d5f) to replace hardcoded strings with a Visibility union type and almost got there 🤓:

    ~/tmp/elm-todomvc ω  elm make Todo.elm
-- BAD FLAGS ---------------------------------------------------------- Todo.elm

Your `main` is demanding an unsupported type as a flag.

29| main =
    ^^^^
The specific unsupported type is:

    Visibility

The types of values that can flow through in and out of Elm include:

    Ints, Floats, Bools, Strings, Maybes, Lists, Arrays, Tuples, Json.Values,
    and concrete records.

Detected errors in 1 module.                                        
    ~/tmp/elm-todomvc ω  

which I only partly understand: I understand that it doesn’t know how to serialize/deserialize Visibility, but I don’t understand why is this required and how to fix it. 🤔

Since it points to the main function declaration, I suspect this may be related to App.programWithFlags requiring that the model passed through update is only made of parts of those types. 🤔

I’m also wondering if this approach makes sense at all. 😊

mez commented 8 years ago

@gurdiga I am still new to Elm but here is my implementation and I used union types. https://github.com/mez/elm-todo/blob/master/Todo.elm#L14

gurdiga commented 8 years ago

@mez Haha! Very neat approach here: https://github.com/mez/elm-todo/blob/master/Todo.elm#L198-L207! 😎

Thank you for sharing!

rofrol commented 7 years ago

This is what I got when tried to use union type for visibility in evancz/elm-todomvc:

Port `setStorage` is trying to communicate an unsupported type.

The specific unsupported type is:
 
     Todo.VisibilityFilter
 
 The types of values that can flow through in and out of Elm include:
 
     Ints, Floats, Bools, Strings, Maybes, Lists, Arrays, Tuples, Json.Values,
     and concrete records
rofrol commented 7 years ago

Probably this is the solution for ports http://stackoverflow.com/questions/37999504/how-to-pass-union-types-through-elm-ports/38006565#38006565 or this https://github.com/elm-lang/persistent-cache will be released

mez commented 7 years ago

@rofrol persistent cache sounds like a nicer solution. I was at elm-hack night yesterday trying to get ports to work on my todo app and to get around the "union types can't go through ports" issue I just stored the todo items in localstorage 🔨