Skip to content

Conversation

pharpend
Copy link

@pharpend pharpend commented Nov 3, 2014

No description provided.

I couldn't get the test suite to build (circular dependencies), else I would
have verified my instances against the tests. `cabal build` works, so I figure I
couldn't have completely ruined it.

The `Eq` instance is a standard `deriving` clause. The `Ord` instance compares
the output of `zonedTimeToUTC`, using `UTCTime`'s `Ord` instance.
@AshleyYakeley
Copy link
Member

Rejected, because this finds "midnight +00:00" == "1am +01:00" to be True, when they are clearly different values of ZonedTime.

Additionally, ZonedTime just isn't morally an instance of Ord. There should not be an "instance Ord ZonedTime".

@pharpend
Copy link
Author

pharpend commented Nov 3, 2014

Rejected, because this finds "midnight +00:00" == "1am +01:00" to be True, when they are clearly different values of ZonedTime.

They are quite obviously the same time.

@AshleyYakeley
Copy link
Member

They're not the same ZonedTime. A ZonedTime isn't precisely a time. It's a time (or a LocalTime) together with a TimeZone. From a ZonedTime you can derive a LocalTime, a TimeZone, and a UTCTime, but a ZonedTime isn't any one of these things.

@hvr
Copy link
Member

hvr commented Nov 3, 2014

They are quite obviously the same time.

This depends on what you consider the exact definition of Eq/Ord (or rather "same time").

And I agree with @AshleyYakeley here that algebraically, ZonedTime is not a totally ordered set, and thus can't have a (total) Ord instance.

If you need to compare two ZonedTimes you can always normalise them and use something like comparing zonedTimeToUTC which makes your semantics much more explicit.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants