Abstract away the interface of Interval from the rest of the module #2
Conversation
|
Ping? |
|
Thank you for your PR, and sorry for the late reply.
Do you worry about moving the definition of |
src/Data/Interval.hs
Outdated
| , -- | 'upperBound' of the interval and whether it is included in the interval. | ||
| -- The result is convenient to use as an argument for 'interval'. | ||
| upperBound' :: !(Extended r, Bool) | ||
| } deriving (Eq, Generic, Data, Typeable) |
There was a problem hiding this comment.
This PR replaces hand-written Data instance with the one generated by GHC.
But doesn’t the generated one allow constructing unnormalized interval value (e.g. through gmapT)?
There was a problem hiding this comment.
I confirmed that following y becomes unnormalized interval where lower bounds is Finite (-1) while upper bound is Finite 0.
{-# LANGUAGE ScopedTypeVariables #-}
import qualified Data.Interval as Interval
import Data.Data
import Data.Maybe
f :: Data a => a -> a
f x = fromJust $ do
(x' :: Interval.Extended Integer, b::Bool) <- cast x
cast (fmap negate x', b)
x :: Interval.Interval Integer
x = 0 Interval.<=..<= 1
y :: Interval.Interval Integer
y = gmapT f x
So could you revert this change?
|
Other changes looks good to me. |
|
Thanks, please take another look. |
|
Thank you very much for your contribution. |
I've been playing with different implementations of
Intervaland it appeared to be useful to do not pattern-match onIntervalconstructor in the body of the module, relying solely onlowerBound'/upperBound'/empty/interval.It would be even better to move definition of
Intervalto a separate module, but it is difficult because ofLatticeinstances.What is your opinion on this?