Sharing Class names: a bad, bad practice….
April 19th, 2008
I consider sharing class names (in design or implementation) a bad, bad practice. In my opinion, it is a practice that will lead to confusion among developers, will eventually cause problems, and will promote software entropy.
You don’t have to go that far to find an example of this bad practice. You can find it right within the Java’s core API
public class Arrays {
// ... some code goes here
public static <T> List<T> asList(T... a) {
return new ArrayList<T>(a);
}
// ... some code goes here
}
Hmm, this method returns an ArrayList. You’d think that you could do the following and get no errors.
public class Gotcha {
public void whatsWrongWithThisA(){
final String[] args = {"a", "b", "c", "d"};
final List<String> luckyList = Arrays.asList(args);
for(Iterator<String> itr = luckyList.iterator();
itr.hasNext();)
{
final String next = itr.next();
System.out.println(next);
itr.remove();
}
Assert.assertTrue(!luckyList.isEmpty());
}
}
You are wrong! You will definitely get an error if you run it.
java.lang.UnsupportedOperationException
at java.util.AbstractList.remove(AbstractList.java:144)
at java.util.AbstractList$Itr.remove(AbstractList.java:360)
at com.gotobject.clearance.Gotcha.whatsWrongWithThisA(Gotcha.java:28)
First of all, the ArrayList that you think is being returned by the Arrays.asList(…) is not the java.util.ArrayList that you and I are so familiar with. This one is a private static nested class that shares the same name as the one in the Collection framework. This nested class uses AbstractList.Itr class as its main iterator. If you examine its code you will notice that this iterator’s remove method invokes the AbstractList’s remove method, which returns an UnsupportedOperationException. I don’t know if this name was intentionally given by its designers, but it does not seem right. Maybe it should have been named FixedArrayList or something like that. Do you think of a better name?
p.s.
Here is the entire code. I’ve include both the test that shows the problem and the test that shows a temporary fix.
public class Gotcha {
@Test(expectedExceptions =
UnsupportedOperationException.class)
public void whatsWrongWithThisA(){
final String[] args = {"a", "b", "c", "d"};
final List<String> luckyList = Arrays.asList(args);
for(Iterator<String> itr = luckyList.iterator();
itr.hasNext();)
{
final String next = itr.next();
System.out.println(next);
itr.remove();
}
Assert.assertTrue(!luckyList.isEmpty());
}
@Test
public void whatsWrongWithThisB(){
final String[] args = {"a", "b", "c", "d"};
final List<String> luckyList = new ArrayList<String>(
Arrays.asList(args)
);
for(Iterator<String> itr = luckyList.iterator();
itr.hasNext();)
{
final String next = itr.next();
System.out.println(next);
itr.remove();
}
Assert.assertTrue(luckyList.isEmpty());
}
}
Leave a Reply