Could you add helper methods for compareTo isBefore and isAfter or similar?

[gissuebot]

Original issue created by lexicalscope on 2011-12-22 at 01:52 PM

---

I would find two methods like:

public static <T> boolean isBefore(Comparable<T> o1, Comparable<T> o2);

public static <T> boolean isAfter(Comparable<T> o1, Comparable<T> o2);

very useful.

[gissuebot]

Original comment posted by wasserman.louis on 2011-12-22 at 05:43 PM

---

Not convinced these increase readability, though other method names could make this more interesting.

[gissuebot]

Original comment posted by lexicalscope on 2011-12-22 at 06:14 PM

---

well I'm not too attached to the exact method names, more just want a single place that encapsulates the "< 0" "> 0" logic (which is kind of hard to read in my opinion and requires you to remember the contract of compareTo).

[gissuebot]

Original comment posted by stephan202 on 2011-12-22 at 06:16 PM

---

How does this relate to the Ranges class? E.g., it seems to me that the first method can be replaced with (or implemented as) one of the following two calls:     Ranges.lessThan(o2).contains(o1);     Ranges.greaterThan(o1).contains(o2);

Arguably 'isBefore' and 'isAfter' are more to the point, but Guava's philosophy seems to be that there should be (precisely) one preferred way to do things.

[gissuebot]

Original comment posted by wasserman.louis on 2011-12-22 at 06:18 PM

---

The advantage of the compareTo approach is that you use a "<=" to express "x <= y", and so on, which is pretty cute.

My first reflex on this issue was "this is a kitchen sink," which Guava is not the place for...but I can see how people could find the current syntax less than readable. I'd like to hear the opinion of some other Guava developers on this.

[gissuebot]

Original comment posted by cgdecker on 2011-12-22 at 09:43 PM

---

I've found (having tried something similar at one point) that "isBefore(a, b)" isn't really much of an improvement in readability because it's not instantly obvious whether it's checking if the first parameter is before the second one or the other way around. Though it's still rather awkward, I like "Ranges.lessThan(b).contains(a)" a little bit better.

What would really work well would be "a.isBefore(b)" or "a.isLessThan(b)" which are much clearer since they read like "a < b". Perhaps JDK8 will add default methods to the Comparable interface for this.

[gissuebot]

Original comment posted by cgdecker on 2011-12-22 at 09:53 PM

---

A weird idea:

  Comparison.check(a).isLessThan(b)

This wraps a in an object that provides these convenience comparison methods. Obviously this somewhat improved readability comes at a small cost which may just make it not worth it at all, I don't know. This could obviously be extended to Comparators as well, possibly through Ordering (e.g. someOrdering.check(a).isLessThan(b)).

The names of both the class and the method are just something I came up with in 30 seconds so it could probably be named better.

[gissuebot]

Original comment posted by michael.hixson on 2011-12-23 at 12:23 AM

---

The weird idea in comment #6 distracted me for a couple of hours. Here's another suggestion on how to name those, if you decide to proceed:

  Comparison.is(a).lessThan(b);   Comparison.using(comparator).is(a).lessThan(b);

API:

  Comparison {     public static <T extends Comparable<? super T>> Subject<T> is(T object);     public static <T> Rule<T> using(Comparator<? super T> comparator);   }

  Comparison.Subject<T> {     public boolean lessThan(T other);     public boolean lessThanOrEqualTo(T other);     public boolean equalTo(T other);     public boolean greaterThanOrEqualTo(T other);     public boolean greaterThan(T other);   }

  Comparison.Rule<T> {     public Subject<T> is(T object);   }

[gissuebot]

Original comment posted by cgdecker on 2011-12-23 at 01:41 AM

---

Nice, I'd also come up with "is(a).lessThan(b)" and "using(comparator).is(a).lessThan(b)" when thinking about the names a little more after posting that. With static imports, it's almost as compact as if Comparable actually had the methods.

[gissuebot]

Original comment posted by michael.hixson on 2011-12-23 at 03:04 AM

---

I had thought about it some more and was leaning towards:

  Comparisons {     public static <T> By<T> by(Comparator<? super T> comparator);     public static <T extends Comparable<? super T>> Is<T> is(T object);   }

...but I hadn't considered static imports. "Comparisons.by(comparator).is(a).lessThan(b)" reads well but "by(comparator).is(a).lessThan(b)" is awkward.

[gissuebot]

Original comment posted by neveue on 2011-12-23 at 06:54 AM

---

As Colin wrote in comment 6, I think it makes more sense to add the method to Guava's Ordering class, instead of obtaining the comparison builder through a static factory method that takes a Comparator as a parameter:

Ordering<String> ordering = ...; return ordering.is(a).lessThan(b);

If all you have is a standard Comparator, you can obtain an Ordering using the Ordering.from(Comparator) static factory method.

[gissuebot]

Original comment posted by wasserman.louis on 2011-12-23 at 10:36 AM

---

For reference, Ordering.from(comparator) is how you convert a general Comparator to our "fluent" Ordering class, which seems like a perfectly good place for this.

I could get behind Ordering.is(a).lessThan(b).

[gissuebot]

Original comment posted by cgdecker on 2011-12-23 at 02:01 PM

---

One issue with just using Ordering for this (which otherwise seems like it'd be an excellent place for it) is that it wouldn't be possible to have "is" as both a static method and an instance method. I think a shortcut to make using it with Comparable types shorter than "Ordering.natural().is(...)" would be important.

[gissuebot]

Original comment posted by cgdecker on 2011-12-24 at 06:16 PM

---

I've added a few commits with a sample of this functionality here: http://code.google.com/r/cgdecker-guava/source/list?name=comparison-helper

The implementation is obviously pretty trivial and I could use some input on the naming and the locations of these methods. I put the static method for use with Comparable instances in a new Comparables class since I haven't thought of any good way to provide it through Ordering yet. Of course, I'm also curious as to whether others on the Guava team consider this a good idea at all.

[gissuebot]

Original comment posted by michael.hixson on 2011-12-25 at 06:26 AM

---

I ended up putting it in "Comparables" too. And then to justify the existence of the new class to myself I dumped pass throughs to all of Ordering.natural's non-factory instance methods in there along with "is". Looks like you were more sane about it.

I used "Ordering.Is" instead of "Ordering.Wrapper" but wasn't thrilled about it. I figured that if people were going to store it in a variable they'd name it like "isA", so "Is<Foo> isA = is(a);" made sense. Also considered Orderable, Comparison, Ordering.Query, Ordering.Question.

It was a fun little project to try to make this work, but... I pictured myself explaining to a code reviewer familiar with Comparable & Comparator why I used this instead of "a.compareTo(b) < 0" and couldn't come up with a good argument. Heck, half the javadoc I wrote in my implementations of this was explaining when not to use it. Oh well. Looking forward to hearing guava devs' thoughts.

[gissuebot]

Original comment posted by wasserman.louis on 2011-12-28 at 06:24 PM

---

I think this is still worth considering, but not too strenously or high-priority.

---

Status: Triaged

[gissuebot]

Original comment posted by kevinb@google.com on 2012-01-10 at 11:57 PM

---

"I pictured myself explaining to a code reviewer familiar with Comparable & Comparator why I used this instead of "a.compareTo(b) < 0" and couldn't come up with a good argument. Heck, half the javadoc I wrote in my implementations of this was explaining when not to use it."

This is exactly how I feel about it. compare/compareTo in conjunction with <= < > >= is the universal Java idiom and I don't think we can really improve on it. We did spend some time trying to, but it's better to just accept the convention we have in this case.

---

Status: WontFix Labels: Type-Enhancement