Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visibility #55

Open
zwhitchcox opened this issue Oct 22, 2016 · 7 comments
Open

Visibility #55

zwhitchcox opened this issue Oct 22, 2016 · 7 comments

Comments

@zwhitchcox
Copy link

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
Copy link

gurdiga commented Oct 27, 2016

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
Copy link

gurdiga commented Oct 28, 2016

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
Copy link

mez commented Nov 13, 2016

@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
Copy link

gurdiga commented Nov 13, 2016

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

Thank you for sharing!

@rofrol
Copy link

rofrol commented Jan 8, 2017

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
Copy link

rofrol commented Jan 8, 2017

@mez
Copy link

mez commented Jan 12, 2017

@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 🔨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants