corsis / clock

High-resolution clock functions: monotonic, realtime, cputime.
Other
58 stars 25 forks source link

[TimeSpec / Num] Documentation #49

Open OlivierSohn opened 6 years ago

OlivierSohn commented 6 years ago

Hello,

In Num instance of TimeSpec, I think some documentation could be added regarding units for fromInteger argument and unit element of multiplication (*):

Aside from this, the reason I had to think about this is because I encoutered a bug in my program after refactoring the time representation from Float (representing time durations using seconds) to TimeSpec : passing "1" to a function taking a TimeSpec was interpreted as one nanosecond, instead of one second before.

Example :

main = test 1 -- was seconds, now nanoseconds

test :: TimeSpec -> IO ()
test = putStrLn . show

Maybe disabling fromInteger, like so, could have prevented the bug (Although I'm not sure if it's a good idea to disable it in the library as it could lead to huge regressions for ppl using it):

  fromInteger = error "please use fromSecs or fromNanoSecs instead"

And I would have been forced to write:

main = test $ fromSecs 1 -- The unit is in the function name so there is less ambiguity for the developper

test :: TimeSpec -> IO ()
test = putStrLn . show

fromSecs n = TimeSpec n 0

My solution sofar is to create a wrapper type where the Num instance is replaced by |+| and |-| operators to provide addition and substraction only.

CetinSert commented 5 years ago

@OlivierSohn - can you send a pull request with the changes you want?