Effective Java

by Joshua Bloch

To describe the access level that applies when none is specified, this book uses the traditional package-private instead of the technically correct package access [JLS, 6.6.1].

@bp

The Collections Framework API is much smaller than it would have been had it exported forty-five separate public classes, one for each convenience implementation. It is not just the bulk of the API that is reduced but the conceptual weight: the number and difficulty of the concepts that programmers must master in order to use the API. The programmer knows that the returned object has precisely the API specified by its interface, so there is no need to read additional class documentation for the implementation class.

>The programmer knows that the returned object has precisely the API specified by its interface This isn't completely true / let's not completely overlook the drawbacks. For example, the List returned by Collections.unmodifiableList does have a "reduced weight" — e.g., there's isn't a different UnmodifiableList class or interface — but that means there's also no static indication that the returned list is unmodifiable. The consumer(s) who it's given to (and it could be passed from one consumer to another) just has to know its unmodifiable (did JavaDoc along each step of the way convey this information?), lest he only find out at runtime that trying to modify it results in an UnsupportedOperationException.

A fifth advantage of static factories is that the class of the returned object need not exist when the class containing the method is written. Such flexible static factory methods form the basis of service provider frameworks

I wish he'd not imply coupling between the service provider pattern and static factories pattern. The service provider pattern is often accomplished without static factories, and I might encourage preferring the non-static route first. edit: <spoiler alert> he touches on this in Item 5. But I think it's worth mentioning explictily here, too.

In short, the telescoping constructor pattern works, but it is hard to write client code when there are many parameters, and harder still to read it. The reader is left wondering what all those values mean and must carefully count parameters to find out.

Plus, for the parameters that don't apply to their situation situation, developers must still choose values and it might not always be obvious what value to provide for those (null? empty string? zero? one? negative one?).

Luckily, there is a third alternative that combines the safety of the telescoping constructor pattern with the readability of the JavaBeans pattern. It is a form of the Builder pattern

A fourth option that would be nice to have in some situations—this nutrition example is a good one—is named parameters with defaults.

A singleton is simply a class that is instantiated exactly once [Gamma95]. Singletons typically represent either a stateless object such as a function (Item 24) or a system component that is intrinsically unique. Making a class a singleton can make it difficult to test its clients because it’s impossible to substitute a mock implementation for a singleton unless it implements an interface that serves as its type.

Also consider whether you should be deciding whether the class should be singleton. If you choose to enforce singleton, you've taken that choice away from your consumers. Are you sure there isn't a use case where more than on instance might make sense? Maybe your consumer wants a "singleton" with one set of parameters, and another singleton with a different set. Or maybe they're adding support for multiple accounts, and want a "singleton" per account. I would suggest defaulting to being flexible about instantiability and let your consumers decide whether to use a singleton of your class. And they often have good tools available to help them with that, like Spring or Dagger.

Nothing that a client does can change this, with one caveat: a privileged client can invoke the private constructor reflectively (Item 65) with the aid of the AccessibleObject.setAccessible method. If you need to defend against this attack, modify the constructor to make it throw an exception if it’s asked to create a second instance.

Don't mistake this for a security measure; this probably isn't enough to defend from adversarial code running within your JVM. In other words, there's a very narrow window in which this mechanism makes sense to employ, and I doubt it'll ever be the right tool for most of you!

They can also be used to group static methods, including factories (Item 1), for objects that implement some interface, in the manner of java.util.Collections. (As of Java 8, you can also put such methods in the interface, assuming it’s yours to modify.)

>you can also put such methods in the interface I'm not sure yet (haven't seen nor tried this, yet), but I don't think I like the idea of a more cluttered interface. That is, I'd err on keeping the utility methods separate.

To improve the performance, explicitly compile the regular expression into a Pattern instance (which is immutable) as part of class initialization, cache it, and reuse the same instance for every invocation of the isRomanNumeral method:

Beware the other edge of this sword: thread-safety (a likely factor when we're talking about "performance critical situations" that call for caching instances). A frequent example of this in practice is DateFormat, where (similar to this example) you want might instantiate an instance with a format pattern once, and re-use it (call `.format()`) many times. However, DateFormat uses internal state (that is, private instance fields whose values change) while formatting. This means that concurrent calls to `.format()` can mess each other up. In this case, you have a number of options: just take the cost of instantiating new objects each use; keep a thread-local copy; synchronize usage of the object, introduce a pool of instances that can be "checked out" for use... See another related note just below on "When an object is immutable, it is obvious it can be reused safely."

When an object is immutable, it is obvious it can be reused safely,

Let's be a bit more careful/skeptical. An object can _appear_ immutable — it might not have setters and it might not be mutable to you — but that doesn't necessarily mean it doesn't have internal state (mutable private fields) which can cause it to not be reusable by multiple threads.

Because memory leaks typically do not manifest themselves as obvious failures, they may remain present in a system for years. They are typically discovered only as a result of careful code inspection

A memory leak that I was once bitten by was a ThreadPoolExecutor. Even if you have fully dereferenced it, it won't get GC'd until you shut it down (by calling `shutdown()`). In other words, beware of this reasonable exception: unreferenced Threads don't get GC'd, even if they're sitting idle.

For example, java.net.URL’s equals method relies on comparison of the IP addresses of the hosts associated with the URLs. Translating a host name to an IP address can require network access, and it isn’t guaranteed to yield the same results over time.

Eww, really?!

When the hashCode method is invoked on an object repeatedly during an execution of an application, it must consistently return the same value, provided no information used in equals comparisons is modified.

Hmm, something I haven't questioned before: If information in the object does change that changes its equals & hashcode, a `get` on the new value will fail, right? Because the object will still be in the old hashcode's bucket. I wonder how much this has actually bitten people...

Short.hashCode(prefix);

`Short` is a thing in Java? So unused I've completely forgotten it exists!

One important decision you’ll have to make when implementing a toString method is whether to specify the format of the return value in the documentation. It is recommended that you do this for value classes, such as phone number or matrix. The advantage of specifying the format is that it serves as a standard, unambiguous, human-readable representation of the object. This representation can be used for input and output and in persistent human-readable data objects, such as CSV files. If you specify the format, it’s usually a good idea to provide a matching static factory or constructor so programmers can easily translate back and forth between the object and its string representation.

tl;dr: Please err on _not_ using toString() for data exchange (whether file/CSV, API response, or database storage), except for few exceptions. Instead, expect that your intended audience of the string is developers (i.e., technical, non-user human working under the hood). Note: This is the 2nd edition of this comment (on the 3rd edition of this book) =). You can see my 1st edition of this comment (on the 2nd edition of this book) here: https://www.goodreads.com/notes/22719418-effective-java/74-robert/93bf3bdf-e20d-4e72-b471-5401411c3ead. Let's start by pointing out that there are three possible audiences for this string: End users, Machines (i.e., data exchange with other code), and Developers (debugging). * End-Users*: toString() is insufficient for preparing a string for display to users, since it cannot reasonably adapt to context; it lacks a good mechanism for formatting for locale, for content around it (e.g., tense, pluralization, etc.), etc.. *Machines, data exchange*: There can easily be a variety of places the string needs to be sent to: file/disk, over the wire (possibly to a variety of clients), or database. Each may want/expect a different format; e.g., ISO 8601, or Unix Timestamp. Even this phone number example is a case-in-point why: There isn't a single phone number format, even in the US. Should you add — do all of your clients expect— the '+1' country code? So, it's best to create explicit methods for each format — better yet, separate the formatter from the object (i.e., create a class or utility method for formatting the data for your use case; like DateFormat). In other words, End-Users and Machines need/deserve something more flexible and predictable than toString(), and why make toString() this thing that might sometimes be tailored to either of those, and sometimes not, and even then perhaps often still not what they need? So, just make toString always tailored for developers to aid in debugging. Among the exceptions are those objects that represent things that are strings by definition, that have only one well-defined, correct string representation. An example of that are URIs; a string-based specification for resource identification.

you are usually better off providing an alternative means of object copying. A better approach to object copying is to provide a copy constructor or copy factory.

This really ought to be in the lead.

If a class has multiple significant fields, the order in which you compare them is critical.

If your class has multiple significant fields, that's a good flag to consider whether the object has natural ordering. For example, even just people's names: sort by first name then last name, or the other way around? Instead, consider forgoing implying that there's a natural ordering (and provide Comparators).

package-private—The member is accessible from any class in the package where it is declared. Technically known as default access, this is the access level you get if no access modifier is specified

On p. 3, he refers to the default access as "the technically correct _package access_ [JLS, 6.6.1]."

Note that a nonzero-length array is always mutable, so it is wrong for a class to have a public static final array field, or an accessor that returns such a field. If a class has such a field or accessor, clients will be able to modify the contents of the array. This is a frequent source of security holes:

Hmm, what about Enum's `public static T[] values()`? I just realized there's no JavaDoc for that method... Does that vend a copy (I never thought about using `values()` as having a performance implication)? Even the JLS doesn't say.

Ensure that the class can’t be extended. This prevents careless or malicious subclasses from compromising the immutable behavior of the class by behaving as if the object’s state has changed. Preventing subclassing is generally accomplished by making the class final, but there is an alternative that we’ll discuss later.

Even with the caveat, I wish this recommendation wasn't so high in the list. Library classes (where `final` is most appropriate to consider) are too often just shy of being valuable to use, and extending them is often a good option for getting the behavior you want. If a developer is in a position to consider extending your class, let them, at their own risk. (More on this in a following note.)

Make all fields final. This clearly expresses your intent in a manner that is enforced by the system.

Unfortunately, modern frameworks — such as many dependency injection frameworks — work against this (can't mark fields that would otherwise be initialized in the constructor and be final, but are initialized by DI). It would be great for an update to the language to allow this.

Recall that to guarantee immutability, a class must not permit itself to be subclassed. This can be done by making the class final, but there is another, more flexible alternative. Instead of making an immutable class final, you can make all of its constructors private or package-private and add public static factories in place of the public constructors (Item 1).

This begins to conflate immutability to protect developers from misusing classes as consumers, with locking down the class to protect it from developers looking to extend functionality. In far too many cases over the years, I've wanted to subclass a class in order take advantage of the classes abilities, but unnecessarily locked down, forcing me to copy/paste a tree of classes. Perhaps a good analogy is cars: designed to shield drivers of the internal operations, and make it easy to use and hard to make mistakes. But many of the screws and bolts are removable; components replaceable and upgradable. Intrepid drivers, if they wish _and at their risk_, may modify their car. Please let us do the same with classes.

The list of rules for immutable classes at the beginning of this item says that no methods may modify the object and that all its fields must be final. In fact these rules are a bit stronger than necessary and can be relaxed to improve performance. In truth, no method may produce an externally visible change in the object’s state. However, some immutable classes have one or more nonfinal fields in which they cache the results of expensive computations the first time they are needed. If the same value is requested again, the cached value is returned, saving the cost of recalculation.

There's a caveat: if you go just a bit further with this, it's possible that usage of internal mutable fields makes the class not thread-safe, as is the case with DateFormat.

In other words, a subclass depends on the implementation details of its superclass for its proper function. The superclass’s implementation may change from release to release, and if it does, the subclass may break, even though its code has not been touched. As a consequence, a subclass must evolve in tandem with its superclass, unless the superclass’s authors have designed and documented it specifically for the purpose of being extended.

Android's `Activities` et al. might be the worst violation of this that I've encountered.

// Reusable forwarding class public class ForwardingSet<E> implements Set<E>

Should be abstract? Instantiating this directly has no value, so may as well prevent that. It would also prevent the confusion of potential consumers about what this class does.

Such tagged classes have numerous shortcomings. They are cluttered with boilerplate, including enum declarations, tag fields, and switch statements. Readability is further harmed because multiple implementations are jumbled together in a single class. Memory footprint is increased because instances are burdened with irrelevant fields belonging to other flavors. Fields can’t be made final unless constructors initialize irrelevant fields, resulting in more boilerplate. Constructors must set the tag field and initialize the right data fields with no help from the compiler: if you initialize the wrong fields, the program will fail at runtime. You can’t add a flavor to a tagged class unless you can modify its source file. If you do add a flavor, you must remember to add a case to every switch statement, or the class will fail at runtime. Finally, the data type of an instance gives no clue as to its flavor. In short, tagged classes are verbose, error-prone, and inefficient.

If you find yourself doing if-else/switch on the same possible set of values more than twice, that's a likely flag that you've got a "tag" that is better off being class hierarchy.

Next, define a concrete subclass of the root class for each flavor of the original tagged class. In our example, there are two: circle and rectangle. Include in each subclass the data fields particular to its flavor. In our example, radius is particular to circle, and length and width are particular to rectangle. Also include in each subclass the appropriate implementation of each abstract method in the root class.

For some cases, an alternative/shortcut of this is to encapsulate the various attributes and behaviors of each flavor in the tag as enum (instead of subclasses). That is, it still might be desirable to enumerate the various flavors (for example, if you expose the possible flavors) and you put the flavor specific implementation ('area()' in this example) in each enum value.

If you declare a member class that does not require access to an enclosing instance, always put the static modifier in its declaration,

A more actionable way to put this suggestion: Consider starting with the static modifier in place, and only remove it when it has become apparent that the class is better off as an inner class.

Before lambdas were added to Java (Chapter 6), anonymous classes were the preferred means of creating small function objects and process objects on the fly, but lambdas are now preferred (Item 42).

We'll get to this, but: Are lambdas much more than syntactic sugar for anonymous classes?

Never put multiple top-level classes or interfaces in a single source file.

Why is this even possible? I thought it was a JLS violation to declare a top-level class with a name that doesn't match the file name? I thought I get a compiler _error_ if I try. Or is this my toolset going above and beyond what the compiler will allow?

From this declaration, the compiler knows that stamps should contain only Stamp instances and guarantees it to be true, assuming your entire codebase compiles without emitting (or suppressing; see Item 27) any warnings.

That guarantee can also be violated if you give your instance to library code (might be 'provided' or otherwise opaque) that doesn't respect your type declaration (that the compiler can't see and warn you about)

Some warnings will be much more difficult to eliminate. This chapter is filled with examples of such warnings. When you get warnings that require some thought, persevere! Eliminate every unchecked warning that you can.

Better yet, use IntelliJ and fix the file until the top-right status square is green!

Every time you use a @SuppressWarnings("unchecked") annotation, add a comment saying why it is safe to do so. This will help others understand the code, and more importantly, it will decrease the odds that someone will modify the code so as to make the computation unsafe.

I know it'd be a bit of a misuse, but it'd be nice if the SuppressWarning annotation had a 'comment' parameter.

To eliminate the unchecked cast warning, use a list instead of an array. Here is a version of the Chooser class that compiles without error or warning:

Funnily enough, you haven't eliminated the problem, you've simply transferred the problem to ArrayList. That's still good as you've moved the problem in capable hands. You do also trade for another caveat, too: It's possible to use a list that doesn't support random access; i.e., you can trade the type safety concern for the possibility of accidental performance problem (for O(n) rather than O(1)).

Once you’ve proved that an unchecked cast is safe, suppress the warning in as narrow a scope as possible (Item 27). In this case, the constructor contains only the unchecked array creation, so it’s appropriate to suppress the warning in the entire constructor.

But why not still encourage putting it on just the statement? I don't see the downside, and then when someone else comes in to make the class better, we don't accidentally mask an error they might introduce.

Objects.requireNonNull(type),

Tangent: I'd love to be understand why Java went the route of Optional rather than @Nullable and @NotNull/@NonNull annotations. Or rather, why is Optional better?

In other words, the Map does not guarantee the type relationship between keys and values, which is that every value is of the type represented by its key. In fact, Java’s type system is not powerful enough to express this. But we know that it’s true, and we take advantage of it when the time comes to retrieve a favorite.

Well, we "know it's true," here, but in a more distant spot (far away when this class becomes large) we could put in a value that doesn't fit the key class, and generics/compiler couldn't protect from that case.

But that’s OK, because the getFavorites method can and does reestablish this linkage.

I wouldn't call that OK. Generics helps provide guarantees between distant code. Here, it does not.

Note that the name of each apple constant is prefixed with APPLE_ and the name of each orange constant is prefixed with ORANGE_. This is because Java doesn’t provide namespaces for int enum groups.

It's a little bit disingenuous to depict int enums this way. You can namespace them with Packages and Classes, which is part of what Enums is also taking advantage of. (that's not to mean that I'm arguing for int enums).

Programs that use int enums are brittle. Because int enums are constant variables [JLS, 4.12.4], their int values are compiled into the clients that use them [JLS, 13.1]. If the value associated with an int enum is changed, its clients must be recompiled. If not, the clients will still run, but their behavior will be incorrect.

Tangent: When is the value of an int "constant" _not_ compiled into clients? Consider the following: private static final int ONE = 1; private static final int TWO = 1 + 1; private static final int THREE = Integer.parseInt("3"); private static final int FOUR; static { FOUR = 4 } I'm guessing only 'ONE' is compiled into clients, but maybe also the 'TWO'? 'FOUR' could be used as a strategy to "hide" values from being compiled into clients so they become runtime-safe? Not advocating, just pointing out strangeness out of this optimization.

For a nice example of a rich enum type, consider the eight planets of our solar system.

Were you sad, Joshua, when you had to remove Pluto between the second and third editions of this book?

While the Planet enum is simple, it is surprisingly powerful. Here is a short program that takes the earth weight of an object (in any unit) and prints a nice table

I almost think it'd be worth showing what this would look like with int enums for contrast!

If you override the toString method in an enum type, consider writing a fromString method to translate the custom string representation back to the corresponding enum.

In line with an earlier soapbox of mine (https://www.goodreads.com/notes/37646821-effective-java/74-robert/cd025f84-7dcb-4402-86a3-13ae1acf2050), I would discourage this. I encourage `toString()` be used exclusively for developers (and not for code/parsing nor end-users), and this proposed counterpart,`fromString`, is intended for code. On top of the reasons in the previous comment, also consider if you may need to maintain compatibility with consumers who might use fromString long after toString (i.e., across running instances), which then prevents you from rename your enums (or you have to refactor to introduce capability to support toString that's different from the enum name). This is a lot like the 'Use instance fields instead of ordinals' item. Instead, either use an external "formatter" (e.g., PlanetEncoder) or at least create explicit methods like toCode and fromCode.

Note that the Operation constants are put into the stringToEnum map from a static field initialization

Why isn't this `STRING_TO_ENUM`?

// The strategy enum pattern enum PayrollDay { MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY(PayType.WEEKEND), SUNDAY(PayType.WEEKEND); private final PayType payType; PayrollDay(PayType payType) { this.payType = payType; } PayrollDay() { this(PayType.WEEKDAY); } // Default

This use of default constructor is hardly less error-prone than the previous example: one can just as easily add HOLIDAY and unintentionally inherit the default strategy.

A minor performance disadvantage of enums is that there is a space and time cost to load and initialize enum types, but it is unlikely to be noticeable in practice.

Android deemed it noticable. Personally, I think these days it's probably safe to assume Enums are usually a better choice (benefit > cost) for most application code.

But it also includes other sets for which you know all the possible values at compile time, such as choices on a menu, operation codes, and command line flags.

Or states of a state machine (like TCP connection status, or user-to-user relationship status).

The Enum specification has this to say about ordinal: “Most programmers will have no use for this method. It is designed for use by general-purpose enum-based data structures such as EnumSet and EnumMap.” Unless you are writing code with this character, you are best off avoiding the ordinal method entirely.

I think it's worth being more precise: ordinal is unsafe to use if order might ever change, and if the value might every be saved and used between changes (e.g., don't use as the value to save in the database).

There is one minor exception to this rule. If you are writing a class that is not labeled abstract and you believe that it overrides an abstract method in its superclass, you needn’t bother putting the Override annotation on that method. In a class that is not declared abstract, the compiler will emit an error message if you fail to override an abstract superclass method.

I still think you should annotate implementations of abstract and interface methods. If in the future the abstract/interface method goes away, your method is now an orphan. With the @Override annotation the orphanization isn't silent; it is called out (as an error) so you can figure out what to do about it.

the Set interface is just such a restricted marker interface. It is applicable only to Collection subtypes, but it adds no methods beyond those defined by Collection. It is not generally considered to be a marker interface because it refines the contracts of several Collection methods, including add, equals, and hashCode. But it is easy to imagine a marker interface that is applicable only to subtypes of some particular interface and does not refine the contracts of any of the interface’s methods. Such a marker interface might describe some invariant of the entire object or indicate that instances are eligible for processing by a method of some other class

> It is easy to imagine a marker interface that is applicable only to subtypes of some particular interface and does not refine the contracts of any of the interface’s methods. Such a marker interface might describe some invariant of the entire object or indicate that instances are eligible for processing by a method of some other class What about RandomAccess, which marks that a list supports random access in O(1) - why not give that as an example rather than leave it to our imagination to come up with one?