Best way to check if all elements in a List are unique

[Lew]

The method already does that - if it's null, you get a
NullPointerException.

Umm, the point of my comment was to *avoid* the NPE, duhh. Why cause trouble
for the method's client?

 

[Mike Schilling]

Umm, the point of my comment was to *avoid* the NPE, duhh. Why cause
trouble for the method's client?

Consistency with all of the Collections methods, which just throw NPEs.

 

[Tom Anderson]

Umm, the point of my comment was to *avoid* the NPE, duhh. Why cause trouble
for the method's client?

I'm hoping that you're being sarcastic. Or that i've misunderstood - i
read your comment as suggesting that the method should null-check the
incoming argument.

The right thing for this method to do if passed a null argument is to
throw an exception. There is no uncertainty about that. And it already
does that, so no check is necessary.

tom

 

[Tom Anderson]

What if there are null entries in the list?

Then you are a bad man, and you do not deserve to be using my code!

If you do need to have nulls in the list, then you can use the
time-honoured and sorely-missed-from-the-standard library formula:

private static boolean equals(Object a, Object b) {
return (a != null) ? a.equals(b) : (b == null);
}

This snippet also, of course, assumes that T is a comparable object (I
believe Collections.sort fails if there's no comparator and T can't be
cast to Comparable).

Certainly true. I should have typed the method accordingly, my apologies.

tom

 

[Mike Schilling]

I'm hoping that you're being sarcastic. Or that i've misunderstood - i
read your comment as suggesting that the method should null-check the
incoming argument.

The right thing for this method to do if passed a null argument is to
throw an exception. There is no uncertainty about that. And it already
does that, so no check is necessary.

Sometimes the check is necessary, of course. Consider
Collections.unmodifiableList(), for which a naive implementation goes
something like

public List unmodifaibleList(List list)
{
class UnmodifiableList implments List
{
private List list;

UnmodifiableList(List list)
{
this.lst = list;
}

public get(int i)
{
return list.get(i);
}

// etc.
}

return new UnmodifiableList(list);
}

That will work, but it's much beter for the UnmodifiableList constructor to
be

UnmodifiableList(List list)
{
if (list == null)
throw new NullPointerException();
this.lst = list;
}

so that the exception is thrown immediately rather than waiting until the
new list is first accessed.

 

[Roedy Green]

List<String> list = new ArrayList<String>();
for (int i = 0; i < 100; ++i)
list.add(new String("unique"));

? Both "one" and "one hundred" are reasonable answers.

Well done! I wanted to explain that, but started tripping over my
words and ended up deleting my explanation, thinking it would confuse
more than elaborate. You nailed it succinctly and clearly.
--
Roedy Green Canadian Mind Products
http://mindprod.com

The major difference between a thing that might go wrong and a thing that cannot possibly go wrong is that when a thing that cannot possibly go wrong goes wrong it usually turns out to be impossible to get at or repair.
~ Douglas Adams (born: 1952-03-11 died: 2001-05-11 at age: 49)

 

[Lew]

I ...
read your comment as suggesting that the method should null-check the
incoming argument.

That's what I recommended, yes.

The right thing for this method to do if passed a null argument is to
throw an exception. There is no uncertainty about that. And it already
does that, so no check is necessary.

Not necessarily.

As Mike pointed out, some action may be needed upon discerning a null. To his
example, for example, I'd add logging in some scenarios. The method may be
one whose contract is to deliver some default value, such as Mike's suggestion
for the OP's case

boolean areListElementsUnique(List<?> l)

that a null argument be deemed trivially as containing no duplicate elements.
There is no reason for that check method to throw an NPE when it could
reasonably return 'true'. If that were the contract, the method would need
the null check. If that were not the contract, but the contract were to throw
NPE or IllegalArgumentException or MyApplicationException, then it would do that.

 

[luc peuvrier]

Umm, the point of my comment was to *avoid* the NPE, duhh.  Why cause trouble
for the method's client?

with this:

<T> boolean areListElementsUnique( List<T> list )
{
if (list == null || list.size() == 1 )
{
return true;
}
return list.size() == new HashSet <T> (list).size();
}

if list is null, areListElementsUnique return true, saying "list with
unique elemen", but list is null, not a single element list.

There is also the empty list case

Luc

 

[Tom Anderson]

That's what I recommended, yes.


Not necessarily.

As Mike pointed out, some action may be needed upon discerning a null. To
his example, for example, I'd add logging in some scenarios. The method may
be one whose contract is to deliver some default value, such as Mike's
suggestion for the OP's case

boolean areListElementsUnique(List<?> l)

that a null argument be deemed trivially as containing no duplicate elements.
There is no reason for that check method to throw an NPE when it could
reasonably return 'true'.

I honestly can't imagine a situation where it would be reasonable for it
to do that. If your design involves passing around nulls to represent
empty lists, or lists in some other way not containing duplicates, then
your design is broken.

tom

 

[Mike Schilling]

That wasn't my suggestion.

I honestly can't imagine a situation where it would be reasonable for
it to do that. If your design involves passing around nulls to
represent empty lists, or lists in some other way not containing
duplicates, then your design is broken.

I agree. A null isn't a List, and should throw an NPE.

 

[Lew]

That wasn't my suggestion.

Oops, I didn't intend that misattribution. I meant that the method was your
suggestion.

Sorry about that, chief.

The point is that it depends on the required contract. Some apps may wish to
consider the empty list (including null) as trivially not violating the
uniqueness constraint. It isn't necessarily an error that the list be null.

And even if it is, as you yourself pointed out you'll wish to handle it at the
point of detection, and I add, at the least you'll want to log it and quite
possibly wrap in a different exception, so it's not possible from one's throne
to decree for all and sundry that they must accept the implicit NPE
willy-nilly. Contrary to Tom's [sic] pronouncement, there very well may need
to be a null check even with a rethrow.

 

[Mike Schilling]

The point is that it depends on the required contract. Some apps may
wish to consider the empty list (including null) as trivially not
violating the uniqueness constraint. It isn't necessarily an error
that the list be null.
And even if it is, as you yourself pointed out you'll wish to handle
it at the point of detection, and I add, at the least you'll want to
log it and quite possibly wrap in a different exception,

This I doubt. The stanndard way to signal an illegally null argument is to
throw an NPE. You see this all through the Java system classes, and in
particular throughout the collections hierarchy.

so it's not
possible from one's throne to decree for all and sundry that they
must accept the implicit NPE willy-nilly. Contrary to Tom's [sic]
pronouncement, there very well may need to be a null check even with
a rethrow.

 

[Tom Anderson]

Oops, I didn't intend that misattribution. I meant that the method was your
suggestion.

Sorry about that, chief.

The point is that it depends on the required contract. Some apps may wish to
consider the empty list (including null) as trivially not violating the
uniqueness constraint.

I agree that some might. But i cannot agree that they should. All that is
required for evil to triumph, i need not remind you, is for good
programmers to write code that tolerates nulls.

It isn't necessarily an error that the list be null.

And even if it is, as you yourself pointed out you'll wish to handle it
at the point of detection,

I will interject quietly to say that Mike's point about detecting nulls
early and dispatching them promptly, rather than waiting for the
inevitable spooky NPE at a distance, was spot on, and i'm a fool for not
having thought of it.

and I add, at the least you'll want to log it

True. Logging is to an extent orthogonal to throwing exceptions, and there
are certainly cases where you want to do both.

and quite possibly wrap in a different exception, so it's not possible
from one's throne to decree for all and sundry that they must accept the
implicit NPE willy-nilly. Contrary to Tom's [sic] pronouncement, there
very well may need to be a null check even with a rethrow.

Alright. But returning a value is not on.

The point about throwing a more specific exception is interesting. In
general, if a method gets a duff argument, we throw an
IllegalArgumentException. But where it's duff because it's null, we throw
NullPointerException. And where it's an index that's out of bounds, we
throw IndexOutOfBoundsException. Why do we make those special cases?
Should we? Or should we reserve those for the VM, and always throw
IllegalArgumentException? Should NPE and IOOBE be subclasses of IAE? Or
should we have a whole range of more precise exceptions to throw? I rarely
see new subclasses of RuntimeException defined for this purpose; should we
be more keen on doing that?

tom

 

[Volker Borchert]

Should NPE and IOOBE be subclasses of IAE?

I don't think so. In some cases, they might refer to an object's internals
and IllegalStateException might seem more appropiate.

 

[Mike Schilling]

I will interject quietly to say that Mike's point about detecting
nulls early and dispatching them promptly, rather than waiting for the
inevitable spooky NPE at a distance, was spot on, and i'm a fool for
not having thought of it.

Don't say that. I don't want to be a fool every time you think of something
I overlooked.

 

[Tom Anderson]

Don't say that. I don't want to be a fool every time you think of something
I overlooked.

TOO BAD!

tom