Use non-empty lists to avoid crashes with empty frequency list etc.?

elm-explorations / test

Write unit and fuzz tests for Elm code.

https://package.elm-lang.org/packages/elm-explorations/test/latest

BSD 3-Clause "New" or "Revised" License

237 stars 39 forks source link

Use non-empty lists to avoid crashes with empty frequency list etc.? #158

Closed drathier closed 3 years ago

drathier commented 4 years ago

Do we want to change the api to use non-empty lists (probably via having function signatures like fn : a -> List a -> ... rather than fn : List a -> ...) to avoid some crashes at runtime?

Main pros:

Avoids Debug.todo.

Main cons:

It's unlikely that a mistake like this would make it into a project, as CI presumably runs tests before the failing test is merged in, so this could be a big non-issue in practice.
There's plenty of other places where we return invalid, failing the test, such as when a frequency weight is 0.

harrysarson commented 4 years ago

If we want to go with this. Presumably we then have to decide between:

fn : a -> List a -> ...
fn : (a, List a) -> ...
type alias NonEmpty a = (a, List a)
type NonEmpty a = NonEmpty a (List a)
Importing mgold/elm-nonempty-list

avh4 commented 4 years ago

Just noting that elm/random went with a -> List a -> ... https://package.elm-lang.org/packages/elm/random/latest/Random#uniform

drathier commented 4 years ago

I think I'd vote against this. It's not going to be safe with this change, but the api will be a bit less composible.