Java Generics and Collections by Maurice Naftalin, Philip Wadler The unconfirmed error reports are from readers. They have not yet been approved or disproved by the author or editor and represent solely the opinion of the reader. Here's a key to the markup: [page-number]: serious technical mistake {page-number}: minor technical mistake : important language/formatting problem (page-number): language change or minor formatting problem ?page-number?: reader question or request for clarification This page was updated August 1, 2008. UNCONFIRMED errors and comments from readers: {7} Second Sentence; States that reference type is any "class, instance, or array type." The Java Specification says that there are three reference types: "class types, interface types, and array types." Therefore replace instance with interface. {7} left column of table of Primitive and Reference types in Java; Primitive type should be boolean, not bool. [7]second code snippet; The line ints.add(new Integer(1)); should be changed to ints.add(Integer.valueOf(1)); That's what Sun's compiler generates (and probably all others). Integer.valueOf() caches small values. The following sentence, which mentions caching, should be changed to explain the use of valueOf(). {12} code snippet, fourth line; the rightmost parenthesis must be removed or else the code won't compile [12]2nd code snippet and 3rd paragraph; Here's the entry in Sun's bug database for the Lists.toList() problem: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6730476 [12]2nd code snippet and 3rd paragraph; The line List ints = Lists.toList(); should be changed to List ints = Lists.toList(); The following paragraph explains why adding "" appears to be necessary: "[W]ithout the type parameter there is too little information for the type inference algorithm to infer the correct type. It infers that the argument to toList is an empty array of an arbitrary generic type rather than an empty array of integers, and this triggers the unchecked warning described earlier." (The latter is probably referring to the "unchecked varargs" warning.) That's not true. It is true that without the type parameter, Sun's javac (1.5 and 1.6) issues a warning: "unchecked generic array creation of type T[] for varargs parameter". But it does not issue an error, which means that it accepts the assignment of the result of Lists.toList() to a variable of type List without a cast, which means that it correctly inferred the type Integer for the type parameter T of method Lists.toList(). That an unchecked warning is issued is actually a bug in Sun's javac. The Eclipse compiler (which generally has fewer generics bugs than javac) compiles the line List ints = Lists.toList(); without any warnings or errors. {16} 3rd code snippet; A list which is created by method Arrays.asList(T...) cannot be manipulated via method add() of interface List. So line 3 in this code snippet would cause a java.lang.UnsupportedOperationException at runtime. I know that this code snippet would never be runnable because of the compile error in line 2 but the rest of the code should be as correct as possible. (16) Last sentence; The order of the two words "List" and "List" in the last sentence of page 16 should be swapped, because it should be saying that List is not a subtype of List. {19} First line; "...which is a subtype of List..." must be read as "...which is a supertype of List..." {19}First line; The following error report (listed on the unconfirmed error reports page) is wrong: "...which is a subtype of List..." must be read as "...which is a supertype of List..." List is indeed a subtype of List. The quoted sentence is correct as printed in the book. (19)line 2; "as required by the super" should be "as required by the super wildcard" [21] Last code snippet - third line; ints.add(3) is ok at compile-time, but fails to execute at runtime. The reason is that the ArrayList object returned by the asList() method in the first line comes from an inner class of the Arrays class, that doesn't implement the add(E e) method, required by the contract of the List interface. So, the call to Arrays$ArrayList.add(E e) always throws an UnsuportedOperation exception. [21] the last code block; package javaapplication38; import java.util.*; public class Main { public static void main(String[] args) throws AssertionError { List objs = Arrays.asList(1, "two"); List ints = objs; ints.add(3); } public static void count(Collection ints, int n) { for(int i = 0; i < n; i++) ints.add(i); } public static double sum(Collection nums) { double s = 0.0; for(Number num : nums) s += num.doubleValue(); nums.add(null); return s; } public static double sumCount(Collection nums, int n) { count(nums, n); return sum(nums); } } init: deps-jar: Compiling 1 source file to C:\Documents and Settings\IOANNIS_PAPAIOANNOU\My Documents\NetBeans_Projects \JavaApplication38\build\classes compile: run: Exception in thread "main" java.lang.UnsupportedOperationException at java.util.AbstractList.add(AbstractList.java:151) at java.util.AbstractList.add(AbstractList.java:89) at javaapplication38.Main.main(Main.java:10) Java Result: 1 BUILD SUCCESSFUL (total time: 0 seconds) {22} second para, after code sample; The section describes the bounding of get and put for generics - "Similarly, you cannot get anything out from a type declared with an extends wildcard...". I believe that this should be changed to a "super wildcard". {23} The 2 code blocks in the middle of the page.; The code is listed as: List ints = Arrays.asList(1,2,3); List nums = ints; nums.put(2, 3.14); List ints = Arrays.asList(1,2,3); List nums = ints; nums.put(2, 3.14); Line 2 of the first block is an intended compiler error, but line 3 on both blocks calls a "puts" method that doesn't appear to be a method of the List interface. I'm assuming a java.util.List is the correct interface. {23}The 2 code blocks in the middle of the page.; In the code snippets, "put" should be changed to "set". {24} 3rd paragraph; "Arrays of primitive type are much more efficient ... assignments into such an array need not check for an array store exception, because primitive types don't have subtypes." As per JLS 3ed 4.10, primitive types ARE arranged in a subtype hierarchy. Array types whose component types are primitive are not arranged in a subtype hierarchy, so a more accurate clause would be "because arrays of primitive type don't have subtypes." {26}Under Type Parameters, code involving MyList; This code has MyList objs = MyList.asList("one", 2, 3.14, 4); MyList ints = MyList.asList(2, 4); I believe both these lines should read .... = Arrays..... instead of ..... = MyList.... {26}final two paragraphs and first paragraph on the following page; This section correctly notes that the library designers consciously chose not to use the type parameter, E, for Collection in the parameters to the following methods: contains, containsAll, remove, removeAll, and retainAll. The section characterizes this as a backwards compatibility choice. This is incorrect. The example of a legacy client using the test "ints.containsAll(objs)" is irrelevant. In a legacy client, objs would have the raw type Collection so the test would compile and run the same no matter which signature was given to the containsAll method in Java 1.5. The erasure of contains(E) is contains(Object) so the method will look the same to a legacy client. There was no backwards compatibility component to this decision. The true reason becomes clear when one compares these methods with the methods add and addAll. These methods do make use of the type parameter E. There is an important difference between these two sets of methods. The add and addAll methods add the argument to the collection while the other methods do not. In order to be type safe, these methods MUST include E in their signature. For the other methods no elements are added so the parameters need not have their types restricted in this way. The last paragraph on page 26 argues that the decision to be more permissive in contains (etc.) might have been the wrong one and claims that if one wants to call contains(E) with an object not of type E one could broaden the type of the collection. While one could argue this point, I think doing so would clearly be too burdensome a requirement. As one example, the client might not control the type of the collection. In that case, if the collection were a known type, at least the client could cast to that type before calling these methods (contains, etc.). However, if the type parameter E is not bound to a known type in the code in question, an unchecked cast to E would be required. By accepting Object instead of E where doing so does not violate type safety the library designers pulled all these considerations inside the implementation, simplifying client code. (27) First and second examples; According to Java Language Specification §8.4, the grammar for method declaration is: MethodHeader: MethodModifiers(opt) TypeParameters(opt) ResultType MethodDeclarator In page 27, there are two examples, which read: public static void reverse(List list) as "void" in the examples is the result type, these examples don't compile. The correct form would be public static void reverse(List list) (29) 4th paragraph; Change "at" to "as" in "elements from the inner lists at any other type". [30] last code snippet; class NestedList implements ArrayList> is not ok! extends instead of implements is probably what the author wanted here {31} 3.1 2nd paragraph "The compareTo method..."; The book contains : "The compareTo method returns a value thatis negative, zero, or positive depending upon whether the **argument** is less than, equal to, or greater then the given object." Something is confusing in this sentence. An extract from the Java API at : http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Comparable.html int compareTo(T o) Compares this object with the specified object for order. Returns a negative integer, zero, or a positive integer as this object is less than, equal to, or greater than the specified object. Is the "given object" the argument o of type T (bad terminology) ? Or is it the reference on which the method is called via the dot operator (contradicts the Java contract) ? (33) 5th last paragraph; The sentence "Every value is compares as the same as itself" does not make sense. [33] after first example; What property is being generalized here? How can x x." (36) A Fruity Example; What are example 3.2 and 3.1? This is not consistent with other code fragments presented in book. {37} Comparator; The text says the Comparator interface contains a single method: public int compare (T o1, t o2); It contains two methods, the other method being: public boolean equals(Object obj); [38] class Fruit; In the examples on pages 38 and 39, the classes define the "hash" method and call the "hash" method on a String. This won't compile. Presumably what was intended was to define and call the "hashCode" method. {41} last paragraph; The text states "By the contract for comparators, it would be equivalent to leave the arguments in the original order but negate the result." This is not quite true if we take the meaning of "negate" to be "apply the unary minus operator". The negation of Integer.MIN_VALUE is not representable in a Java int. The value that results from applying the unary minus operator to Integer.MIN_VALUE is Integer.MIN_VALUE. Since Integer.MIN_VALUE is a legal return value of Comparator.compareTo, a comparator which simply applies the unary minus operator to the result of another comparator's compareTo method is not guaranteed to represent a reverse ordering of that comparator. {42}last code snippet, 2nd line; The parameter declaration "Comparator comp" should be changed to "Comparator comp" to make the method applicable in more cases. [47]code snippets; The 'copy' method does not close the given streams if an exception occurs. In general, streams should be closed in 'finally' blocks. Additionally, they should be closed in the code block where they were opened, because otherwise even a 'finally' block is not really reliable. For example, if the third line in the second code snippet throws an exception, the stream opened in the second line will remain open. In a nutshell: A code block should close the streams it opens, but no others. (48)2nd paragraph from bottom; The paragraph explains the code example 3.6, but has the "first" and "second" method the wrong way around. {50}4th paragraph; According to the Java Language Specification, 3rd ed, §8.4.2, the signature of a method does not include its return type. In the first sentence, "if the signatures match exactly" should be changed to "if the signatures and return types match exactly" In the second sentence, "if the arguments match exactly" should be changed to "if the signatures match exactly" [54]code snippet in middle of page; The code snippet contains the method public static int getCount() { return count; } This should be changed to public static synchronized int getCount() { return count; } The paragraph following the code snippet states "getCount returns the current count", but without synchronization, getCount may return an old value. See the 8th line from bottom on page 161: "Each thread may use a separate memory cache, which means that writes by one thread may not be seen by another unless either they both take place within blocks synchronized on the same lock, or unless the variable is marked with the 'volatile' keyword." P.S.: In this case, a better solution than 'synchronized' or 'volatile' would be java.util.concurrent.atomic.AtomicInteger. {58} ninth bullet point; "The erasures of S and T in the definition of copy are Appendable and Readable, because S has bound Appendable & Closeable and T has bound Readable & Closeable." This is incorrect, S and T have been switched. S has bound Readable & Closeable. T has bound Appendable & Closeable (see page 47). So it should read : "The erasures of S and T in the definition of copy are Readable and Appendable, because S has bound Readable & Closeable and T has bound Appendable & Closeable." {59} sample listing Overloaded2; Not really a technical mistake, more of an issue in editing. The text says: "However, say we change the methods so that each appends its result to the end of the argument lis rather than returning a value.", then the listing for Overloaded2, with two methods: public static boolean allZero(List list) and public static boolean allZero(List list). However, neither of those methods: - are changed versions of previous methods (the previous example used sum() methods that returned a sum of integers or concatenated Strings); - appends anything to the argument list. {63}code for ArrayStack.java; The line public boolean empty() { return list.size() == 0; } should probably be changed to public boolean empty() { return list.isEmpty(); } {65}code for ArrayStack.java; The line public boolean empty() { return list.size() == 0; } should probably be changed to public boolean empty() { return list.isEmpty(); } {65}code for Stacks.java; The line public static Stack reverse(Stack in) should probably be changed to public static Stack reverse(Stack in) to make the method applicable in more situations. {81}4th paragraph; As far as I know, C# generics are reified, so the statement "unchecked casts in C# are much more dangerous than unchecked casts in Java" is wrong - there are no unchecked casts in C#. {93}7th line from bottom; In the line newInstance(a.getClass().getComponentType(), c.size()); the expression "c.size()" should be replaced by "size". (Probably copy & paste error, see the code snippet on page 87.) {100}1st paragraph; The statement "it often is more robust to use equality (the equals method) [than the == operator]" is true in general, but wrong if only Class objects are concerned (as seems to be implied in this paragraph), because java.lang.Class does not overide equals(). If cls1 is a java.lang.Class object, cls1 == cls2 is always equivalent to cls1.equals(cls2), even if multiple class loaders are involved. {103} Chapter 7.3 paragraph 3; Printing History: October 2006 First Edition. The line says "java.lang.reflect.array.newInstance(int.class,size)", however the class "java.lang.reflect.array" does not exist in Java 5.0. It should be java.lang.reflect.Array.newInstance(int.class,size). Futher, the next paragraph says "... the call returns a value of type int[]". Actually the above call returns the value of type Object, which can be casted to int[]. (120)3rd line from bottom; "by the sublist method" should be "by the subList method". [125]code for SimpleName and ExtendedName; SimpleName and ExtendedName break the contract for Comparable as spelled out on page 33: if x.compareTo(y) == 0 then sgn(x.compareTo(z)) == sgn(y.compareTo(z)) If s is a SimpleName and e1 and e2 are ExtendedNames, it's possible that s.compareTo(e1) == 0 but s.compareTo(e1) == 0 and e1.compareTo(e2) != 0 Essentially the same problem - applied to equals() instead of compareTo() - is discussed in "Effective Java", Item 7 (1st ed) / Item 8 (2nd ed), "Obey the general contract when overriding equals". {132}2nd paragraph from bottom; Throwable is a checked exception, so the sentence "An exception is checked if it is a subclass of Exception but not a subclass of RuntimeException" should be changed to "An exception is checked if it is not a subclass of RuntimeException or Error" (133) 10th line from page's top (conting blank lines too); In class Functions, method List applyAll(List list) signature should be missing a second parameter: Function f and the code 2 lines below should be ...result.add(f.apply(x)); [133] Entire page; Example 9.5 on page 133 does not match the text surrounding it and does not compile. The text describes a single abstract class. The example is an interface and a separate class with static methods. Additionally, the applyAll method is written as if it were part of the an abstract Function class (as described in the text), but it is actually a non-static method of the separate Functions class. It is referenced in a static context by the Functions class. [139]last code snippet; "abstract class Trust" should be changed to "abstract class Trust>" {154} Example 11.1; Example 11.1 is supposed to be "just about the simplest possible example of how Iterable can be directly implemented." But it's clearly simpler to implement Iterator and Iterable together. We can then avoid constructing an instance of an anonymous class: class Counter implements Iterator, Iterable { private int count; private int i = 0; public Counter (int count) { this.count = count; } public boolean hasNext() { return i < count; } public Integer next() { i++; return i; } public void remove() { throw new UnsupportedOperationException(); } public Iterator iterator() { return this; } } (170) last paragraph; "used to by ordered collections" should read "used by ordered collections" {170} top of page; List l = Array.asList(0,1,2); should be List l = Arrays.asList(0,1,2); (176) first paragraph of section 12.3; "We will go on to look at these three main kinds" "these" does not refer to anything mentioned previously. Better would be "We will go on to look at the three main kinds" {181} Figure 13.3; The curved arrow starting at 'a' should point to 'b' instead of to 'j' and the curved arrow from 'j' to 'b' should point in the reverse direction. (182) second full paragraph; In the sentence "These are snapshot iterators; they reflect the state of the set at the time it was created," the pronoun "it" should be referring to each iterator, but that's not how the English reads. The english suggests that iterators reflect the state of the set at the time the set was created, rather than at the time each iterator was created. (192) code snippets at bottom of page; Some of the source markup is leaking through into the rendered text. The words "verb $NavigableSet$" and "verb$TreeSet$" should presumably be unadorned (i.e. just "NavigableSet" and "TreeSet"). {207} 4th paragraph; Both occurances of "null" in this paragraph should say "false" instead. {207} last paragraph; The text states that "the only way of ensuring that variable writes made by one thread are visible to reads by another is for both writes and reads to take place within blocks synchronized on the same locks." I think this is incorrect. Declaring the variable as volatile should also provide the same assurance. (227) 1st paragraph of section 15.3; The sentence "Even though the choice here is much narrower than with lists or even sets, ..." probably should be "Even though the choice here is much narrower than with queues or even sets, ..." [245] under 16.5.1; The book states iterators (of concurrent Map implementations - not clear, the section is about ConcurrentSkipListMap, but in previous sections nothing is said about iterator of ConcurrentHashMap) are fail-fast. According to Javadoc, they are weakly consistent: http://java.sun.com/javase/6/docs/api/java/util/concurrent/ConcurrentSkipListSet.html "Iterators are weakly consistent, returning elements reflecting the state of the set at some point at or since the creation of the iterator. They do not throw ConcurrentModificationException." http://java.sun.com/javase/6/docs/api/java/util/concurrent/ConcurrentHashMap.html "Iterators and Enumerations return elements reflecting the state of the hash table at some point at or since the creation of the iterator/enumeration. They do not throw ConcurrentModificationException." {247ff} Chapter 17; While chapter 17 gives the impression that all the methods of the java.util.Collections class are described, two methods seem to be missing: Enumeration enumeration(Collection c) ArrayList list(Enumeration e)