avh4 / elm-color

Standard representation of colors, encouraging sharing between packages. (This replaces elm-lang/core#Color from Elm 0.18.)
BSD 3-Clause "New" or "Revised" License
23 stars 6 forks source link

Should fromHex return a Maybe/Result, or just a value? #13

Open avh4 opened 6 years ago

avh4 commented 6 years ago

Currently, fromHex is fromHex : String -> Color, meant for convenience in getting colors specified in hex into Elm, and returns rgb 0 0 0 if the hex string is invalid. Should it instead return a Maybe or Result so it can clearly indicate when a parse error occurs? Or should both functions be provided?

Questions:

2mol commented 5 years ago

To cast my vote:

  1. It should not return a Result, since the details of the error are at most useful for debugging for the library author. We can trust people to find out what's wrong with their string at first glance.
  2. I should return a Maybe, since it is both correct and easy to use with Maybe.withDefault if necessary. In addition, I can myself specify my own default color, like some ugly and very visible pink in those cases where I expect that parsing could fail.

My opinion for 1. is contingent on the function accepting all valid combinations of 3 and 6-digit strings, upper- and lowercase, as well as with or without "#"

avh4 commented 5 years ago

Note: yes, the current implementation support 3-, 4-, 6-, and 8-digit inputs both with and without initial "#", and both uppercase and lowercase hex digits are allowed.

avh4 commented 5 years ago

There were other questions related to fromHex/toHex (like how to deal with alpha, what scenarios do people want to use them, and the questions here). Also, toCssString has been added to dev-1.0.0. So I removed fromHex/toHex from dev-1.0.0, and would like to get some more info about them before adding them back to make sure we pick the right API.

ymtszw commented 5 years ago

Recently I adopted elm-color and prepared ColorExtra to add missing APIs: https://github.com/ymtszw/zephyr/blob/master/src/ColorExtra.elm And I ended up adding fromHex : String -> Result String Color, accompanied by fromHexUnsafe : String -> Color.

Regarding to fromHex,

Just my two cents, but hope it helps.

skyqrose commented 5 years ago

I would use this most often in decoders or other places that are handling untrusted / user inputed data. Using Maybe/Result makes sense in those contexts, and I tend to be doing error handling anywhere, so having a Maybe isn't a burden.

Even if I would just choose a default, that default might not be black. (It could be white, if I'm inputting a background color, or obnoxious pink for debugging.) The library doesn't know enough about my domain to choose a reasonable default.

If the library did try to choose a default, there'd be no way to detect and backtrack to a Maybe type or choose a different color (without making it impossible to intentionally input the default color). But going from a Maybe to a default is very easy.

(and +1 to 2mol's reasons for Maybe instead of Result)