ProgramTest.advanceTime signature

[allanderek]

The ProgramTest.advanceTime function accepts an Int representing the number of milliseconds to advance the time by. However, both Process.sleep(in elm/core) and SimulatedEffect.Process.sleep take the number of milliseconds as a Float. I guess this is because the simulation time is stored as an Int, but does mean less than a millisecond, even though it is possible to sleep for that length of time.

I'm not sure if it is worth it to instead store the simulation time as a Float and then everything is just in floats. I could imagine some argument against that.

Final point, the documentation for advanceTime mentions Task.sleep, but it means Process.sleep.

[avh4]

Yeah, I wanted to avoid any unexpected behavior with time due to rounding. Though if browsers do support higher-than-millisecond resolution for sleeps in a way that's observable, then I'd be open to changing the simulation to track time at a higher resolution. I guess possibly the public API could take a Float even if the internals track things as Ints, though I think I'd want to see a real-world example or two where that difference was important.

And thanks for the additional notes about the docs too.

[allanderek]

I certainly don't have a real-world example. It's probably not worth it to change your design until you see a real need for it. The API came up for me because I had a constant for the delay of a particular effect, so I wanted to just write that, as in ProgramTest.advanceTime Debounce.delay ... and instead I had to write ProgramTest.advanceTime (round Debounce.delay) .... So it had nothing to do with actually wanting 'higher-than-millisecond' precision. Though my test was actually failing for an unrelated reason which meant I briefly changed this to be ProgramTest.advanceTime (1000 + round Debounce.delay) ... just to rule out something going wrong with the interplay between floats and ints.

I guess I think the API would be a bit more consistent accepting a Float, but it's certainly arguable, in particular if it is a 'Float', it's non-obvious that the simulation is using Integers of milliseconds behind the scenes. That could lead to confusion/frustration for a consumer who actually does have a higher-than-millisecond resolution requirement.

[avh4]

Ah yeah, having a delay constant is a good example, and probably the most common one that folks will run into.

Side note: Originally I had considered having advanceTime take a Duration value (from the elm-units package), but decided that would be a bit tedious for folks who don't already use that package in their production code, and for people who do use it, calling Duraiton.inMilliseconds isn't so tedious. But I guess even there, you'd get back a Float instead of an `Int.

I guess I went with having advanceTime take an Int because I strongly wanted to avoid having something unexpected happen due to Floats since that would be quite confusing to debug... but really the decision should be based on that risk weighed relative to how easy the API is to use in the normal case. Maybe the chance of something confusing going wrong with Floats is small enough that it's not worth worrying about? I guess I could still have the internals use Int, and also use ceiling instead of round when converting from Float to Int which would ensure that the requested about of time is always advanced (and if it's not the exact amount you asked for, then it's always more). What's the worst thing that could happen if it were done that way?

Let me think on this a bit, but I think you might be right about this.