6

I am thinking about something like this:

public static <T extends Comparable<T>> T minOf(T...ts){ SortedSet<T> set = new TreeSet<T>(Arrays.asList(ts)); return set.first(); } public static <T extends Comparable<T>> T maxOf(T...ts){ SortedSet<T> set = new TreeSet<T>(Arrays.asList(ts)); return set.last(); }

But is not null safe, which is something I want too.

Do you know a better way to solve this problem?

EDIT:

After the comments I have also tried min():

public static <T extends Comparable<T>> T minOf(T...ts){ return Collections.min(Arrays.asList(ts), new Comparator<T>(){ public int compare(T o1, T o2) { if(o1!=null && o2!=null){ return o1.compareTo(o2); }else if(o1!=null){ return 1; }else{ return -1; } }}); }

What do you think of that?

Improve this question
5
  • That's inefficient since you need O(n log n) compares, since the TreeSet effectively sorts the Collection, where n compares suffice. Also, you create a lot of unneccessary garbage (Arrays.asList creates a copy of "ts", and the TreeSet is not lightweight, too). Commented Dec 15, 2008 at 19:23
  • Arrays.asList does not make a copy of the array. Commented Dec 15, 2008 at 19:34
  • Yes that would work, but why not declare and name that comparator else where so its reusable? Then instead of using this minOf function, just call the standard Collections.min with that named comparator. Your code becomes much more readable by using standard library calls. Commented Dec 15, 2008 at 19:44
  • I would say to use Arrays.sort then check the elements near the front and back, but it throws a NullPointerException if the array contains a null element. Commented Dec 15, 2008 at 19:51
  • for best results you want to use <T extends Comparable<? super T>> Commented Jun 9, 2011 at 10:13

3 Answers 3

Reset to default
38

What's wrong with Collections.max?

And why do you care about null safety? Are you sure you want to allow nulls to be in your Collection?

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

The question was for max and min. Collections.max() will look at every element. Then doing Collections.min() will again look at every element. So maybe sort just once is faster.
@Yetti99 sorting will be slower because it is O(n log n) and max + min is O(2n). What would be a little better is to do a single loop and update both max and min in it. And the OP does not seem to require both in a single function.
@Ciro Santilli 六四事件 法轮功 纳米比亚胡海峰 You are right about average complicity of a sort. But if a list is nearly in order the complexity will be O(n) best case. And then its quite possible that the application may need to have the list sorted for some other reason.
5

If you really need to exclude "null" from the result, and you can't prevent it from being in your array, then maybe you should just iterate through the array with a simple loop and keep track of the "min" and "max" in separate variables. You can still use the "compare()" method on each object to compare it with your current "min" and "max" values. This way, you can add your own code for checking for nulls and ignoring them.

EDIT: here's some code to illustrate what I'm talking about. Unfortunately there is an edge case you need to consider - what if all of the arguments passed in are null? What does your method return?

public static <T extends Comparable<T>> T minOf(T...ts){ T min = null; for (T t : ts) { if (t != null && (min == null || t.compareTo(min) < 0)) { min = t; } } return min; } public static <T extends Comparable<T>> T maxOf(T...ts){ T max = null; for (T t : ts) { if (t != null && (max == null || t.compareTo(max) > 0)) { max = t; } } return max; }
Improve this answer

Comments

1

You should not implement Comparable to accept null, as it breaks the interface's contract.

From https://docs.oracle.com/javase/7/docs/api/java/lang/Comparable.html :

Note that null is not an instance of any class, and e.compareTo(null) should throw a NullPointerException even though e.equals(null) returns false.

You must instead create a new interface, e.g. ComparableNull instead.

See also:

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.