Skip to content
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

[TimeSpec / Num] Documentation #49

Open
OlivierSohn opened this issue Jan 21, 2018 · 1 comment
Open

[TimeSpec / Num] Documentation #49

OlivierSohn opened this issue Jan 21, 2018 · 1 comment

Comments

@OlivierSohn
Copy link

OlivierSohn commented Jan 21, 2018

Hello,

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

  • fromInteger interprets the integral as nanoseconds
  • the unit element of multiplication (*) is "one second" (TimeSpec 1 0)

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.

@OlivierSohn OlivierSohn changed the title Documentation on multiplication between TimeSpec Documentation on Num units Jan 21, 2018
@OlivierSohn OlivierSohn changed the title Documentation on Num units [TimeSpec / Num] Documentation Jan 21, 2018
@CetinSert
Copy link
Member

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

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

No branches or pull requests

2 participants