Check parameters before execution as soon as possible.
Add in public methods @throw, and use assertions in non public methods
Do it also in constructors.
You must program defensively, with the assumption that clients of your class will do their best to destroy its invariants.
//Broken "immutable" time period
public final class Period{
private final Date start;
private final Date end;
/**
* @param start the beginning of the period
* @param end the end of the period; must not precede start;
* @throws IllegalArgumentException if start is after end
* @throws NullPointerException if start or end is null
*/
public Period(Date start, Date end) {
if(start.compare(end) > 0)
throw new IllegalArgumentException(start + " after " + end );
this.start = start;
this.end = end;
}
public Date start(){
return start;
}
public Date end(){
return end;
}
...
}Attack. Because the client keep a copy (pointer) of the parameter, it can always change it after the constructor.
Date start = new Date();
Date end = new Date();
Period p = new Period(start, end);
end.setYear(78)// Modifies internal of p!Make a defensive copy of each mutable parameter to the constructor.
public Period(Date start, Date end) {
this.start = new Date(start.getTime());
this.end = new Date(end.getTime());
if(start.compare(end) > 0)
throw new IllegalArgumentException(start + " after " + end );
}Defensive copies are made before checking the validity of the parameter (Item 38), and the validity check is performed on the copies rather than on the originals. It protects the class against changes to the parameters from another thread during the time between the parameters are checked and the time they are copied.(Window of vulnerability,time-of-check/time-of-use TOCTOU attack)
Do not use clone method to make a defensive copy of a parameter whose type is subclass-able by untrusted parties.
Second Attack. Because the accessors returns the object used in the Period class, the client can change its value without passing the constrains.
Date start = new Date();
Date end = new Date();
Period p = new Period(start, end);
p.end.setYear(78)// Modifies internal of p!Return defensive copies of mutable internal fields.
public Date start(){
return new Date(start.getTime());
}
public Date end(){
return new Date(end.getTime());
}Preferable is to use immutable objects(Item 15)
- Choose method names carefully. (Item 56)
- Don't go overboard in providing convenience methods. Don't add too many.
- Avoid long parameter list. Make a subset of methods, helper classes (Item 22), or a builder (Item 2) instead.
- For parameter types, favor interfaces over classes (Item 52) No reason to write a method that takes a HashMap on input, use Map instead.
- Prefer two-element enum types to boolean parameters.
public enum TemperatureScale {CELSIUS, FARENHEIT}
The choice of which overloading to invoke is made at compile time. Selection among overloaded methods is static, while selection among overridden methods is dynamic.
// Broken! - What does this program print?
public class CollectionClassifier {
public static String classify(Set<?> s) {
return "Set";
}
public static String classify(List<?> lst) {
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>(),
new HashMap<String, String>().values()
};
for (Collection<?> c : collections)
System.out.println(classify(c)); // Returns "Unknown Collection" 3 times
}
}Overriding works different. The “most specific” overriding method always gets executed.
class Wine {
String name() { return "wine"; }
}
class SparklingWine extends Wine {
@Override String name() { return "sparkling wine"; }
}
class Champagne extends SparklingWine {
@Override String name() { return "champagne"; }
}
public class Overriding {
public static void main(String[] args) {
Wine[] wines = {
new Wine(), new SparklingWine(), new Champagne()
};
for (Wine wine : wines)
System.out.println(wine.name()); // prints: wine, sparkling wine, and champagne
}
}Overloading does not give the functionallity we want in the first sample. A possible solution is:
public static String classify(Collection<?> c) {
return c instanceof Set ? "Set" :
c instanceof List ? "List" : "Unknown Collection";
}Do not have overloaded methods in APIs to avoid confusing the clients of the API.
A conservative policy is to never export two overloadings with the same number of parameters. Use different names.writeBoolean(boolean), writeInt(int), and writeLong(long)
For constructors you can use static factories (Item 1)
If parameters are radically different this rule can be violated but always ensure that all overloadings behave identically when passed the same parameters. To ensure this, have the more specific overloading forward to the more general.
public boolean contentEquals(StringBuffer sb) {
return contentEquals((CharSequence) sb);
}varargs methods are a convenient way to define methods that require a variable number of arguments, but they should not be overused.
// The right way to use varargs to pass one or more arguments
static int min(int firstArg, int... remainingArgs) {
int min = firstArg;
for (int arg : remainingArgs)
if (arg < min)
min = arg;
return min;
}There is no reason ever to return null from an array- or collection-valued method instead of returning an empty array or collection
Return an immutable empty array instead of null.
// The right way to return an array from a collection
private final List<Cheese> cheesesInStock = ...;
private static final Cheese[] EMPTY_CHEESE_ARRAY = new Cheese[0];
/**
* @return an array containing all of the cheeses in the shop.
*/
public Cheese[] getCheeses() {
return cheesesInStock.toArray(EMPTY_CHEESE_ARRAY);
}In Collections emptySet, emptyList and emptyMap methods do the same job.
// The right way to return a copy of a collection
public List<Cheese> getCheeseList() {
if (cheesesInStock.isEmpty())
return Collections.emptyList(); // Always returns same list
else
return new ArrayList<Cheese>(cheesesInStock);
}To document your API properly, you must 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, what the method does rather than how it does its job.
- The preconditions described implicity by the @throws tag.
- The postconditions, things that will be true after the invocation has completed successfully.
- The side effects, change in the state of the system.
- The thread safety of the class and methods
- The summary description of the element, the first “sentence” of each doc comment.Should not contains a space after a period.
/**
* Returns the element at the specified position in this list. *
* <p>This method is <i>not</i> guaranteed to run in constant
* time. In some implementations it may run in time proportional * to the element position.
*
* @param index index of element to return; must be
* non-negative and less than the size of this list
* @return the element at the specified position in this list
* @throws IndexOutOfBoundsException if the index is out of range
* ({@code index < 0 || index >= this.size()})
*/
E get(int index);Have special care in:
- Generics: document all type parameters
- Enums: document all the constants, the type and the public methods.
- Annotatons: document all members an the type.
Don't forget to documment: