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

Should negative inputs to `every`, `fps`, `fpsWhen`, `delay` be forbidden? #312

Closed
jvoigtlaender opened this Issue Jul 28, 2015 · 2 comments

Comments

Projects
None yet
3 participants
@jvoigtlaender
Contributor

jvoigtlaender commented Jul 28, 2015

Yes, I think they should. It would be trivial and harmless to make them runtime errors. That would be in the same category like the runtime error one currently gets from calling mergeMany on an empty list. (That is, such runtime errors can only happen immediately at program start. So no sudden surprises.)

In addition to what's in the issue title, probably the value 0 should also be forbidden as argument at least for fps and fpsWhen. (It might effectively already be forbidden for those due to "division by zero".)

@TheSeamau5

This comment has been minimized.

Show comment
Hide comment
@TheSeamau5

TheSeamau5 Jul 28, 2015

Contributor

Or, we could clamp them to some logical minimum value (like 0 or 1 depending on the function) and make sure this behavior is clearly stated in the docs. This could eliminate the runtime errors there.

We could even go so far as to change the type of mergeMany to Signal a -> List (Signal a) -> Signal a to further eliminate the runtime errors there.

Contributor

TheSeamau5 commented Jul 28, 2015

Or, we could clamp them to some logical minimum value (like 0 or 1 depending on the function) and make sure this behavior is clearly stated in the docs. This could eliminate the runtime errors there.

We could even go so far as to change the type of mergeMany to Signal a -> List (Signal a) -> Signal a to further eliminate the runtime errors there.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Sep 11, 2015

Contributor

I have reconsidered and think it would indeed be best to simply clamp the arguments to every, fps, fpsWhen and delay to some logical minimum value as suggested by @TheSeamau5. Making negative (or maybe even zero) values to them a runtime error could backfire like in https://github.com/elm-lang/core/issues/381#issuecomment-137169302.

Contributor

jvoigtlaender commented Sep 11, 2015

I have reconsidered and think it would indeed be best to simply clamp the arguments to every, fps, fpsWhen and delay to some logical minimum value as suggested by @TheSeamau5. Making negative (or maybe even zero) values to them a runtime error could backfire like in https://github.com/elm-lang/core/issues/381#issuecomment-137169302.

@evancz evancz closed this May 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment