Date.Time.t -> Date.time #14
Conversation
| module Time = { | ||
| type t = float | ||
| } | ||
| type time = float |
There was a problem hiding this comment.
This is a confusing name for what this actually is. It isn't any more or less a "time" than t, just a different representation. The closest proper name for the format I think is "Unix time".
There was a problem hiding this comment.
To be explicit, maybe it could be called millisSinceEpoch.
There was a problem hiding this comment.
Yeah, I think that would be better. Since it's no longer a module I don't think it matters much that it's a bit wordy, although ideally I think it also ought to be a private type so that it's distinct from a float, which suggests it should be a module so that it can also have a conventional constructor function. But communicating the concept clearly is more important.
There was a problem hiding this comment.
Can also add more information about what it is exactly in a docstring.
The module Time seems overkill.