Remove typeclass from interval constructor #605
Conversation
| def apply(t: T) = contains(t) | ||
| def &&(that: Interval[T]) = intersect(that) | ||
| def intersect(that: Interval[T])(implicit ord: Ordering[T]): Interval[T] | ||
| def apply(t: T)(implicit ord: Ordering[T]) = contains(t) |
There was a problem hiding this comment.
Let's make apply and && final
| * TODO: It might be good to have types for these properties in algebird. | ||
| */ | ||
| def mapNonDecreasing[U: Ordering](fn: T => U): Interval[U] | ||
| def mapNonDecreasing[U](fn: T => U): Interval[U] |
There was a problem hiding this comment.
Think we need a test for mapNonDecreasing.
There was a problem hiding this comment.
So a law might be: forAll intervals and values t, if we map t and the interval, contains returns the same value.
To make the function non-decreasing we will need to do something like make a new fn and apple the first and if the result is less return the input.
| */ | ||
| def toLeftClosedRightOpen(implicit s: Successible[T]): Option[Intersection[InclusiveLower, ExclusiveUpper, T]] = { | ||
| implicit val ord = lower.ordering | ||
| def toLeftClosedRightOpen(implicit s: Successible[T]): Option[Intersection[InclusiveLower, ExclusiveUpper, T]] = |
There was a problem hiding this comment.
Let's try to see if we can just jump Successible and Predecessible up to Ordering. I can't think of a good reason to have Partial there.
| * that are contained in this Intersection | ||
| */ | ||
| def leastToGreatest(implicit s: Successible[T]): Iterable[T] = { | ||
| def leastToGreatest(implicit s: Successible[T], ord: Ordering[T]): Iterable[T] = { |
There was a problem hiding this comment.
don't need ord now that Successible has an Ordering right?
| * that are contained in this Intersection | ||
| */ | ||
| def greatestToLeast(implicit p: Predecessible[T]): Iterable[T] = { | ||
| def greatestToLeast(implicit p: Predecessible[T], ord: Ordering[T]): Iterable[T] = { |
| * The law is: | ||
| * prev(t) | ||
| * .map { n => partialOrdering.lteq(n, t) && (!partialOrdering.equiv(t, n)) } | ||
| * .map { n => ordering.lteq(n, t) && (!ordering.equiv(t, n)) } |
There was a problem hiding this comment.
this can be more succinct now: prev(t).map(ordering.lt(_, t)).getOrElse(true)
| * The law is: | ||
| * next(t) | ||
| * .map { n => partialOrdering.lteq(t, n) && (!partialOrdering.equiv(t, n)) } | ||
| * .map { n => ordering.lteq(t, n) && (!ordering.equiv(t, n)) } |
There was a problem hiding this comment.
similar: next(t).map(ordering.lt(t, _)).getOrElse(true)
200139d to
d9deab1
Compare
Current coverage is 80.64% (diff: 73.21%)@@ develop #605 diff @@
==========================================
Files 110 110
Lines 4924 4940 +16
Methods 4468 4502 +34
Messages 0 0
Branches 456 438 -18
==========================================
+ Hits 3951 3984 +33
+ Misses 973 956 -17
Partials 0 0
|
Moves the
Orderinginputs from the constructor to the methods that need it, as discussed in #600 .