Effective java : Methods and Generics

Objective: This section discusses aspects of method designs: how to treat parameters and return values, how to design method signatures, and how to document methods. This section also tells you how to maximize the benefits and minimize the complications of generics.

Key topics:

  1. Parameter Validity
  2. Defensive Copies
  3. Overloading
  4. Varargs
  5. Empty and Null Returns
  6. Optional Returns
  7. Generics and Raw Types
  8. Generics and Unchecked Warnings
  9. Generics and Arrays and Lists
  10. Generic Types
  11. Generic Methods
  12. Generics and Bounded Wildcards
  13. Generics and Varargs
  14. Generics and Typesafe Heterogeneous Containers
  15. Exposed APIs and Documentation

Estimated time: 15-30 minutes.

Parameter Validity

Which of the following implementations would be preferable to return a non-negative  BigInteger  for a modulus method?

 Handle exceptions

public BigInteger mod(BigInteger m) {
if (m.signum() <= 0) {
throw new ArithmeticException(“Modulus must be positive: ” + m);
}
… // Do the computation
}

 No need to handle exceptions

public BigInteger mod(BigInteger m) {
… // Do the computation
}

You should think about what restrictions exist on parameters of a method or constructor when you implement them. You should enforce them, and better document them, with explicit checks at the beginning of the method/constructor body. This modest work will be paid back with interest the first time a validity check fails.

Defensive Copies

Can you spot at least one vulnerability of one of the following implementations?

 Use defensive copies

public final class Period {
private final Date start;
private final Date end;

public Period(Date start, Date end) {
this.start = new Date(start.getTime());
this.end = new Date(end.getTime());
if (this.start.compareTo(this.end) > 0) {
throw new IllegalArgumentException(this.start + ” cannot be after ” + this.end);
}
}
public Date start() { return new Date(start.getTime()); }
public Date end() { return new Date(end.getTime()); }
… // Remainder omitted
}

 Don’t use defensive copies

public final class Period {
private final Date start;
private final Date end;

public Period(Date start, Date end) {
if (start.compareTo(end) > 0) {
throw new IllegalArgumentException(start + ” cannot be after ” + end);
}
this.start = start;
this.end = end;
}
public Date start() { return start; }
public Date end() { return end; }
… // Remainder omitted
}

In general, you must program defensively, with the assumption that clients of your class/method will do their best to destroy its variants. In the example above,  Date  is unfortunately mutable, and thus vulnerable even though  start  and  end  are declared as final. For example, a client can easily create a valid  Date  object and modify  end  to be before  start , thus violating its immutability and validity. So, here are several lessons learned from the above broken implementation:

  •  Date  is obsolete and should no longer be used in new code; use  Instant  instead if you are using Java 8+.
  • In the above repaired constructor, defensive copies are made before checking the validity of the inputs, and the validity check is performed on the copies rather than on the originals. This is necessary to protect classes against changes to the inputs from another thread during the window of vulnerability between the time the inputs are checked and the time they are copied.
  • Do not use the  clone  method to make a defensive copy of a parameter whose type is subclassable by untrusted parties, because we don’t know how cloning is implemented in subclasses of that type.

Note, however, that if the cost of defensive copies is prohibitive and the class trusts its clients not to modify its components inappropriately then defensive copies may be replaced with clear documentation.

Overloading

Consider the following program:

public class CollectionClassifier {
public static String classify(Set<?> s) { return “Set”; }

public static String classify(List<?> l) { return “List”; }

public static String classify(Collection<?> c) { return “Unknown collection”; }

public static void main(String[] args) {
Collection<?>[] collections = { new HashSet<String>(), new ArrayList<BigInteger>() };
for (Collection<?> c : collections) {
System.out.println(classify(c));
}
}
}

What does this program print out?
 “Set” and “List”
 “Unknown collection” two times

You must use overloading judiciously because the choice of which overloading to invoke is made at compile time. So, in the above example the program prints “Unknown collection” two times because the compile-time type of the parameter passed to  classify  in the loop is always  Collection<?> . Here are some other considerations when designing overloading methods:

  • Never export multiple overloading methods with the same number of parameters because they are difficult to use. You can always give methods different names, instead, such as  writeBoolean(boolean b)  writeInt(int i)  writeLong(long l) .
  • For constructors, you don’t have the option of using different names, but you can use static factories, instead, such as  Instant toInstant(long time) .

Varargs

Which of the following implementations would be desirable?

 Use args only

static int min(int… args) {
if (args.length == 0) {
throw new IllegalArgumentException(“Must have at least one argument”);
}
int min = args[0];
for (int i = 1; i < args.length; i++) {
if (args[i] < min) {
min = args[i];
}
}
return min;
}

 Use firstArg and remainingArgs

static int min(int firstArg, int… remainingArgs) {
int min = firstArg;
for (int arg : remainingArgs) {
if (arg < min) {
min = arg;
}
}
return min;
}

 Use overloading methods

static int min(int firstArg) {
return firstArg;
}

static int min(int firstArg, int secondArg) {
return Math.min(firstArg, secondArg);
}

static int min(int firstArg, int secondArg, int thirdArg) {
return Math.min(Math.min(firstArg, secondArg), thirdArg);
}

static int min(int firstArg, int secondArg, int thirdArg, int… remainingArgs) {
int min = this.min(firstArg, secondArg, thirdArg);
for (int arg : remainingArgs) {
if (arg < min) {
min = arg;
}
}
return min;
}

The first implementation fails at runtime only if the client invokes this method with no arguments, OUCH! It is also ugly. The second implementation is acceptable for many use cases. In performance-critical situations, however, note that every invocation of the second implementation causes an array allocation and initialization. So, if you have empirical evidence that you can’t afford this cost but you need the flexibility of varargs and you know, for example, that 95 percent of the method invocations have three or fewer parameters, then the third implementation would be a lifesaver.

Empty and Null Returns

Which of the following implementations would be desirable?

 Empty return

private final List<Person> employees = …;

public List<Person> getEmployees() {
employees.isEmpty() ? Collections.emptyList() : new ArrayList<>(employees);
}

 Null return

private final List<Person> employees = …;

public List<Person> getEmployees() {
return employees.isEmpty() ? null : new ArrayList<>(employees);
}

Never return  null  in place of an empty array or collection because it will require clients to check  null  return for all method calls, which is ugly; if they forget to do so then it may cause null-pointer problems that are difficult to debug. Similarly, use  Strings.EMPTY  instead of  null  wherever possible.

Optional Returns

Which of the following implementations would be desirable?

 Throw exception if empty

public static <E extends Comparable<E>> E max(Collection<E> c) {
if (c.isEmpty()) {
throw new IllegalArgumentException(“Empty exception”);
}
E result = null;
for (E e : c) {
if (result == null || e.compareTo(result) > 0) {
result = Objects.requireNonNull(e);
}
}
return result;
}

 Optional return

public static <E extends Comparable<E>> Optional<E> max(Collection<E> c) {
if (c.isEmpty()) {
return Optional.empty();
}
E result = null;
for (E e : c) {
if (result == null || e.compareTo(result) > 0) {
result = Objects.requireNonNull(e);
}
}
return Optional.of(result);
}

Since Java 8, an Optional-returning method is possible, more flexible, and easier to use than one that throws an exception; it is also less error-prone than one that returns  null . Here are some best practices when using  Optional :

  • Never return a null value from an Optional-returning method because doing so defeats the entire purpose of the facility.
  • Use helpers provided by the facility, such as  String lastWordInLexicon = max(words).orElse(“No words…”);  Toy myToy = max(toys).orElseThrow(ToyException::new); .
  • Container types, including collections, maps, streams, arrays, and optionals, should not be wrapped in optionals, because they have already provided facility to handle empty values.
  • Never return an optional of a boxed primitive type, with possible exception of  Boolean  Byte  Character  Short  Float . For other boxed primitive types, use  OptionalInt  OptionalLong  OptionalDouble  instead.

In summary, if you are writing a method that can’t always return a value and it is important for clients to consider that possibility every time they call it, then you should probably return an optional. Note, however, that there are real performance consequences associated with returning optionals; so, for performance-critical methods it may be better to return  null  or throw an exception.

Generics and Raw Types

Can you spot bugs in both implementations below? Which one helps you discover the bug at compile time?

 Use of raw types


private final Collection stamps = …;

stamps.add(new Coin( … ));

for (Iterator i = stamps.iterator(); i.hasNext(); ) {
Stamp stamp = (Stamp) i.next();

}

 Use of generics


private final Collection<Stamp> stamps = …;

stamps.add(new Coin( … ));

for (Iterator i = stamps.iterator(); i.hasNext(); ) {
Stamp stamp = (Stamp) i.next();

}

In the first implementation, adding a coin to the collection results in a vague warning  unchecked call  and only throws a  ClassCastException  at run time, OUCH! In the second implementation, an  error: incompatible types: Coin cannot be converted to Stamp  is displayed at compile time and can help you fix the bug easily and as soon as possible. So, using generics can help you improve the safety and expressiveness of your code. There are, however, exceptions where you cannot use parameterized types, such as  List.class  instead of  List<String>.class , or  if (o instance of Set)  instead of  if (o instance of Set<String>) ; but in those cases using generics doesn’t help but is just noise.

Generics and Unchecked Warnings

Which of the following implementations would be preferable? Why?

 Use unchecked warning in the method scope

public class ArrayList<E> extends AbstractList<E>
implements List<E>, RandomAccess, Cloneable, java.io.Serializable {

@SuppressWarnings(“unchecked”)
public <T> T[] toArray(T[] a) {
if (a.length < size) {
return (T[]) Arrays.copyOf(elementData, size, a.getClass());
}
System.arraycopy(elementData, 0, a, 0, size);
if (a.length > size) {
a[size] = null;
}
return a;
}

}

 Use unchecked warnings in the narrowest possible scope

public class ArrayList<E> extends AbstractList<E>
implements List<E>, RandomAccess, Cloneable, java.io.Serializable {

public <T> T[] toArray(T[] a) {
if (a.length < size) {
@SuppressWarnings(“unchecked”) T[] result = (T[]) Arrays.copyOf(elementData, size, a.getClass());
return result;
}
System.arraycopy(elementData, 0, a, 0, size);
if (a.length > size) {
a[size] = null;
}
return a;
}

}

Unchecked warnings are important; don’t ignore them because every unchecked warning has the potential to throw a  ClassCastException  at runtime. You should do your best to eliminate as many of them as possible. If you can’t, however, get rid of an unchecked warning, but you can prove that the code that provoke it is typesafe then suppress it with the corresponding annotation in the narrowest possible scope.

Generics and Arrays and Lists

Which of the following implementations would be the best in general? Why?

 Don’t use generics

public class Chooser {
private final Object[] choices;

public Chooser(Collection c) { choices = c.toArray(); }

public Object choose() {
Random r = ThreadLocalRandom.current();
return choices[r.nextInt(choices.length)];
}
}

 Use arrays and generics

public class Chooser<T> {
private final T[] choices;

public Chooser(Collection<T> c) { choices = (T[]) c.toArray(); }

public T choose() {
Random r = ThreadLocalRandom.current();
return choices[r.nextInt(choices.length)];
}
}

 Use lists and generics

public class Chooser<T> {
private final List<T> choices;

public Chooser(Collection<T> c) { choices = new ArrayList<>(c); }

public T choose() {
Random r = ThreadLocalRandom.current();
return choices.get(r.nextInt(choices.size()));
}
}

The first implementation will work but clients have to cast the return value of the choose method to a desired type every time they invoke the method, otherwise the cast will fail at runtime. The second implementation will work too but you get a warning at compile time, meaning that the compiler can’t prove if it works. You could prove it yourself, but you’re better off eliminating the cause of the warning. The third implementation would be desirable; it may be a little verbose, and maybe a tad slower, but you won’t get  ClassCastException  at runtime, which is worth it for the peace of mind!

Generic Types

Which of the following implementations would be preferable? Why?

 Use generic types

public class Stack<E> {
private static final int INITIAL_CAPACITY = 16;
private E[] elements;
private int size = 0;

@SuppressWarnings(“unchecked”);
public Stack() { elements = (E[]) new Object[INITIAL_CAPACITY]; }

public void push(E e) {
ensureCapacity();
elements[size++] = e;
}

public E pop() {
if (size == 0) {
throw new EmptyStackException();
}
E e = elements[size–];
elements[size] = null;
return e;
}

}

 Use object-based collections

public class Stack {
private static final int INITIAL_CAPACITY = 16;
private Object[] elements;
private int size = 0;

public Stack() { elements = new Object[INITIAL_CAPACITY]; }

public void push(Object o) {
ensureCapacity();
elements[size++] = o;
}

public Object pop() {
if (size == 0) {
throw new EmptyStackException();
}
Object o = elements[size–];
elements[size] = null;
return o;
}

}

In general, generic types are safer and easier to use than types that require casting in clients’ code. Keep in mind, however, that generic types don’t work with primitives such as  int . In addition, in performance-critical situations the above generic-based implementation may cause heap pollution because the runtime type of the array doesn’t match its compile-time type. In this case, you could declare  elements  as an array of  Object  and add  @SuppressWarnings  before  E e = elements[size–];  of  pop()  method.

Generic Methods

Which of the following implementations would be preferable? Why?

 Use generic types

public static <E> Set<E> union(Set<E> s1, Set<E> s2) {
Set<E> result = new HashSet<>(s1);
result.addAll(s2);
return result;
}

 Use raw types

public static Set union(Set s1, Set s2) {
Set result = new HashSet(s1);
result.addAll(s2);
return result;
}

The second implementation compiles successfully but with two warnings while the first one compiles successfully without any warning. So, like generic types, generic methods are safer and easier to use then those that require their clients to put explicit casts on input parameters and return values. Note that, for generic methods you must add the type parameter list between a method’s modifier and its return type.

Generics and Wildcards

Which of the following implementations would be more flexible? Why?

 Use generic types

public class Stack<E> {

public Stack() { … }
public void push(E e) { … }
public E pop() { … }
public boolean isEmpty { … }

public void pushAll(Iterable<E> src) {
for (E e : src) {
push(e);
}
}

public void popAll(Collection<E> dst) {
while (!isEmpty()) {
dst.add(pop());
}
}
}

 Use wildcards

public class Stack<E> {

public Stack() { … }
public void push(E e) { … }
public E pop() { … }
public boolean isEmpty { … }

public void pushAll(Iterable<? extends E> src) {
for (E e : src) {
push(e);
}
}

public void popAll(Collection<? super E> dst) {
while (!isEmpty()) {
dst.add(pop());
}
}
}

It is clear that for maximum flexibility you should use wildcard types on input parameters that represent producers or consumers. For  pushAll method above, the second implementation enables its clients to pass any  Iterable  whose elements is a subtype of E, whereas clients of the first one can pass only  Iterable  whose elements is a strict type of E. Similarly for  popAll  method, the second implementation enables its clients to pass any  Collection  whose elements is a super type of E, whereas clients of the first one can pass only  Collection  whose elements is a strict type of E. Here are some best practices while working with wildcards:

  • Remember PECS, which stands for Producer-Extends and Consumer-Super. Note that  Comparable  and  Comparator  are always consumers.
  • Don’t use bounded wildcard types as return types, which would force clients to use wildcards in their code.
  • If clients of a class have to think about wildcard types then there is probably something wrong with its APIs.
  • If a type parameter appears only once in an API declaration then replace it with a wildcard; if it is an unbounded type parameter then use unbounded wildcard and if it is a bounded one then use a bounded wildcard.

Generics and Varargs

Can you spot a runtime error in one of the following implementations?

 Use generic types

static <T> List<T> pickTwo(T t1, T t2, T t3) {
switch(ThreadLocalRandom.current().nextInt(3)) {
case 0: return List.of(T1, T2);
case 1: return List.of(T1, T3);
case 2: return List.of(T2, T3);
}
throw new AssertionError(“Oops! Should never be here.”);
}

public static void main(String[] args) {
List<String> colors = pickTwo(“Red”, “Green”, “Blue”);

}

 Use varargs

static <T> T[] toArray(T… args) {
return args;
}

static <T> List<T> pickTwo(T t1, T t2, T t3) {
switch(ThreadLocalRandom.current().nextInt(3)) {
case 0: return toArray(T1, T2);
case 1: return toArray(T1, T3);
case 2: return toArray(T2, T3);
}
throw new AssertionError(“Oops! Should never be here.”);
}

public static void main(String[] args) {
String[] colors = pickTwo(“Red”, “Green”, “Blue”);

}

The second implementation would not generate any warning at compile time. It throws, however, a  ClassCastException  at runtime, OUCH! The reason is that  toArray  method creates an array of type  Object[]  to store  args  and returns that array, which results in that exception when converting that array to an array of type  String[] . So, varargs is powerful in some situations but they don’t interact well with generics because the varargs facility is a leaky abstraction that is built on top of arrays and arrays have different type rules from generics. If you write a method with generic varargs then ensure that the method is type safe and annotate it with  @SafeVarargs  so that is is not unpleasant to use. Here are some other notes to consider:

  • It is unsafe to store a value in a generic varargs array parameter.
  • It is unsafe to give another method access to a generic varargs array parameter, unless if the other method that is correctly annotated with  @SafeVarargs  or if the method is a non-varargs method and it merely computes some function of the contents of the array. Note that  List.of  used in the first implementation is a varargs method but it is a well-designed, correctly-safe-annotated one.

Generics and Typesafe Heterogeneous Containers

Which of the following implementations would be preferable?

 Use generics

public class Favorites {
private Map<Class<?>, Object> favorites = new HashMap<>();

public <T> void putFavorite(Class<T> type, T instance) {
favorites.put(Objects.requireNonNull(type), instance);
}

public <T> T getFavorite(Class<T> type) {
return type.cast(favorites.get(type));
}
}

 Don’t use generics

public class Favorites {
private Map<Class<?>, Object> favorites = new HashMap<>();

public void putFavorite(Class<?> type, Object instance) {
favorites.put(Objects.requireNonNull(type), instance);
}

public Object getFavorite(Class<?> type) {
return favorites.get(type);
}
}

Both implementations let you create a map of favorite values to a variety of classes such as  String.class  Integer.class . The first one would be preferable because it ensures that the value of a map item is an instance of the type stored in its key, and it doesn’t require clients to cast the return value of  getFavorite .

Note that a limitation of both implementations is that it cannot be used for a non-reifiable type such as  List<String>  because there isn’t a Class object for  List<String> . That is a good thing because if both  List<String>.class  and  List<Integer>.class  were legal and shared the same object reference  List.class  then it would break the havoc with the internals of a  Favorites  object. Unfortunately, there is no entirely satisfactory workaround for this limitation.

Exposed APIs and Documentation

If an API is to be usable, it must be documented carefully. Here are some best practices:

  • Precede every exported class, interface, constructor, method, and field declaration with a doc comment.
  • The doc comment for a method should describe succinctly the contract between the method and its client. You must respect that contract forever after the API is distributed wide and far.
  • The doc comment should be readable both in the source code and in the generated documentation. So, spend some time to read the generated documentation.
  • Multiple members or constructors in a class or interface should not have the same summary description.
  • When documenting a generic type or method, be sure to document all type parameters.
  • When documenting an enum type, be sure to document the constants as well as the type and any public methods.
  • Whenever or not a class or static method is thread-safe, you should document its thread-safety level.
  • If a class is serializable then you should document its serialized form.

Leave a Reply

%d bloggers like this: