search

dm3 / clojure.java-time

Java 8 Date-Time API for Clojure
MIT License
461 stars 45 forks source link

`not-after?` not semantically equivalent to `<=` despite documentation implying otherwise #98

Closed nessdoor closed 7 months ago

nessdoor commented 1 year ago

The documentation for not-after? recites:

Like before?, except returns true if the inputs are equal.

The documentation for before? states:

Returns true if time entities are ordered from the earliest to the latest (same semantics as <), otherwise false.

This seems to imply that not-after? should be semantically equivalent to <=, but it is not when invoked like:

(not-after? i)
=> false

If semantically equivalent, this s-expression should have returned true. Note that before? is, in fact, semantically equivalent to <:

(before? i)
=> true

This issue seems to be a consequence of how not-after? is defined as the complement of after?, as the latter is consistent with the behavior of >:

(after? i)
=> true

The same applies to not-before?.

Not sure if this is intended behavior or not, but I found it rather confusing. In addition, when one of these functions is apply-ed, the client code is forced to explicitly check for the arity 1 case and override the result of the evaluation, like so:

(if (> (count coll) 1)
     (apply not-after? coll)
     true)
frenchy64 commented 1 year ago

Good catch. The zero arities don't make much sense to me, but I'd rather go a deprecation route than fixing them. Perhaps we can introduce a couple of new predicates, say, chronological? and reverse-chronological? with the <= semantics?

frenchy64 commented 1 year ago

@nessdoor thoughts on https://github.com/dm3/clojure.java-time/pull/99?

nessdoor commented 1 year ago

I think it's fine to deprecate and replace, if the current name doesn't make sense for the new semantics. But I have no experience with public library design, so I can't provide any further suggestions on that.

I will provide more feedback under the PR directly, so that we don't disperse things.

nessdoor commented 1 year ago

Oh, right, would it be impractical to make all affected types implement ordering as part of their interfaces? Because the problem could be entirely side-stepped, at that point.

pjstadig-sovasage commented 7 months ago

There's another way that not-after? is broken, and that's when given more than two arguments.

(jt/not-after? (jt/local-date 2023 11 14) (jt/local-date 2023 11 12) (jt/local-date 2023 11 13))
;=> true

It seems that not-after? really only works with exactly two arguments.

(jt/not-after? (jt/local-date 2023 11 14) (jt/local-date 2023 11 12))
;=> false

This is because (after? a b c) is really (and (after? a b) (after? b c)) and simply inverting that changes it to (or (not (after? a b)) (not (after? b c))) which is not the same.

The same issue exists with not-before?