Restructure span and duration maybe

darrenldl commented 3 years ago

See issue #25 for discussion

gasche commented 3 years ago

The first idea that comes to mind would be to have

module Span : sig
  type t = private { s : int64; ns : int }
  type error = ...
  val make : s:int64 -> ns:int -> (t, error) result
end
module Duration : sig
  type t = private { sign: sign; ... }
  type error = ...
  val make : sign:sign -> ... -> (t, error) result
end

type t (* abstract *)

val of_span : Span.t -> t
val make_span : s:int64 -> ns:int -> (t, Span.error) result
val to_span : t -> Span.t

val of_duration : Duration.t -> t
val make_duration : sign:sign -> ... -> (t, Duration.error) result
val to_duration : Duration.t -> t

val ( +! ) : t -> t -> t
...

(Note: it's a minor point but I prefer to avoid shadowing ( + ), which people expect to be able to use at integers, and there would be reasons to compute integers within a local open of this module. But I suppose you have a difference preference, which is also fine.)

Obviously you probably need to brainstorm on the names a bit more: if the common module is called Span, then having a submodule Span is not so nice. I thought about calling them Low and Human, but I'm not sure about it.

darrenldl commented 3 years ago

Aha right, moving them under another "root" type...interesting...

But then this means you still have to do one transition from Duration.t to t before doing arithmetic, which isn't too different from transitioning from Duration.t to Span.t?

Well it is cleaner from a design point of view...hm...

darrenldl commented 3 years ago

What if instead of having Duration at all, we add a human friendly constructor for Span.t (of similar signature to Duration.make), say Span.make_human_friendly.

And we introduce a "view" type which is basically Duration.t, and a corresponding Span.view_human_friendly : Span.t -> Span.human_view?

(With better naming etc)

darrenldl commented 3 years ago

Moved Duration into Span.For_human with changes for ergonomics (completed by https://github.com/daypack-dev/timere/commit/4249abee9de27caa09088a1e6efd6dec4cc7f052).

Let me know if you spot any issues.

daypack-dev / timere

Restructure span and duration maybe #27