CHAPTER 11 CONCURRENCY
Item 83: Use lazy initialization judiciously

// Double-check idiom for lazy initialization of instance fields
private FieldType getField() {
	FieldType result = field; // (1)
	if (result == null) { // First check (no locking)
		synchronized (this) {
			if (field == null) // Second check (with locking)
				field = result = computeFieldValue(); // (2)
		}
	}
	return result;
}

This code is broken, because when one thread enters the line (2), other threads may have already read the field into result (line (1)), and it is null, so, when they enter the line (2), they see that field isn't null anymore, because it was already initialized by the first thread so, the method called by those non-first threads, when they start almost immediately after the first thread, returns null in spite that the field was initialized.

To fix this we need to reassign result to the field value again.

Here is a solution that works:

private FieldType getField() {
	FieldType result = field;
	if (result == null) { // First check (no locking)
		synchronized (this) {
			if (field == null) // Second check (with locking)
				field = result = computeFieldValue();
			else
				result = field;
		}
	}
	return result;
}

Or this one, which I like even more, because we check if result is not null first, which happens more often, or to be more precise, always after first initialization.

private FieldType getField() {
	FieldType result = field;
	if (result != null)
		return result;
	synchronized (this) {
		result = field;
		return result != null ? result : (field = computeFieldValue());
	}
}

Attached code demonstrates the error and how the fixes work.

I'm writing this not to accuse (everybody can mistake) Joshua Bloch, whom I respect much and find his book very useful, but to suggest to programmers to use copy-paste technique judiciously.

Thank you.

Lazy.zip

On lambdas and streams chapter there is this exercise:

The problem here is that variable words refer to a stream of lines so the program is processing lines not words. I present a solution using flatMap on words where the line is splitted with a regex(I assumed the words are separated in the file with space, comma, tab, point, dash).

In following two lines,
https://github.com/jbloch/effective-java-3e-source-code/blob/master/src/effectivejava/chapter8/item55/Max.java#L28-L29

• The e.compareTo(result) > 0 part is evaluated in if block.
• And then the requireNonNull(e) part follows in that block.

Do we have any sense for the requireNonNull call?
An NPE may be thrown in the e.compareTo condition, I think.

Shouldn't it be,

//            if (result == null || (e != null && e.compareTo(result) > 0))
//                result = e;

Or, when each e in the collection is lazily checked for the null,

//            if (result == null || Objects.requireNonNull(e).compareTo(result) > 0)
//                result = e;

in item 37 on page 173, the second paragraph talks about "an EnumMap version and two stream-based versions". This is confusing and misleading because the second of the two "stream versions" also happens to use EnumMap, i.e. it's a combination of EnumMap and stream. What the book calls "the EnumMap version" probably refers to the one on 172, which uses only EnumMap and not stream.

Hence, instead of "EnumMap version", the book should probably say "non-stream EnumMap version". You could argue that juxtaposing "EnumMap version" next to "stream-based" automatically implies that the former is non-stream, but I feel like that's still somewhat misleading and made me do a distracted double-take.

I wanted check the item 24 while reading chapter 4 and I expected to find it under chapter 4. I can see item 23 and 25 but not 24.

Not everyone can afford Intellij. please add the following to your gitIgnore:

eclipse specific git ignore

.pydevproject
.project
.metadata
bin/*
tmp/**
tmp/**/*
*.tmp
*.bak
*.swp
*~.nib
local.properties
.classpath
.settings/
.loadpath
/.classpath
/.project

Hello Josh, how do I contact you regarding copy rights?
My company considers using parts of your book.

Dear Joshua,

In example class Intern and method intern is race condition between line 18 and 19. It can also lead to memory leak.

Kindly regards,
Marcin

PS
Thank you for your book. I'm continuously learning new things from it.

Hi Josh,
First of all its an honor to write to you.

Secondly I was hoping if you can point out some more classes like Float,Arrays and other classes where there is an overridden equals and hashCode method.

Also where can I find the native implementation for equals and hashCode method for Linux platofrm

I think there is one point about toString() which could be worth being mentioned in Item 12.

• You should not misuse toString() for displaying objects in a user interface, since that will preclude localization. Rather use java.text.MessageFormat or something similar for that purpose.

This is also highlighted in the text which says that phone numbers have a standard string representation. This "standard" is actually only valid in US, Canada and the other countries with country code 1. Elsewhere in the world, phone numbers looks different.

...and certainly not a package-private one.

In Item 2, the abstract Pizza class declares an abstract Builder class. This abstract builder declares an abstract
build method: abstract Pizza build();. I argue that this is unnecessary and incorrect.

The problem is that by declaring the abstract build method without checked exceptions, the abstract builder class is
forbidding subclasses from throwing checked exceptions from the build method.

The builder pattern is being used in place of constructors here. Having a build method that prevents subclasses from
throwing checked exceptions is like having a constructor that prevents subclasses from throwing checked exceptions in
their constructors.

It is incorrect of the author of the abstract builder class to assume that subclasses would not wish to throw checked
exceptions from their build methods or from the constructors these build method call.

I argue that declaring the abstract build method in the abstract builder is unnecessary because it is not the concern
of the author of the abstract builder class. The author of the abstract builder class should be concerned with writing
setters for values required by the constructor and retrieving these values in the constructor of the target class.

If a subclass of the builder does not have a build method that results in the builder being passed to the target type
constructor, then it is a failure on the part of the subclass, not the abstract builder superclass.

I acknowledge that constructors should be used to construct objects, not do extensive work. As such, we should probably
be mostly throwing RuntimeExceptions from constructors because of validation errors. However, sometimes constructors
do throw checked exceptions (like FileOutputStream). We may also wish to do work which may throw a checked exception
in the build method itself before calling the constructor of a subclass.

I also acknowledge that declaring the abstract build method helps to create subclasses of the builder with
consistently named build methods. Note that because the abstract build method is declared with package-private
access, this consistency is only probable (subclass authors will probably do the logical thing and override it with a
public access modifier - but they may do something weird), and not really that useful. The method is not part of a
consistent API shared by all instances of builders. All instances of builders probably have a build method, but other
classes cannot call Pizza.Builder.build(), only NyPizza.Builder.build() etc. You cant do this:

Collection<Pizza.Builder<?>> l = something();
l.stream().map(Pizza.Builder::build)

With package-private access, the abstract build method is not useful enough to warrant forbidding subclasses from
throwing checked exceptions.

If a consistent API across subclasses of Pizza.Builder is required then the build method should be declared
public in the abstract builder, and the author of the abstract builder should consider allowing subclasses to throw
checked exceptions.

I think authors of abstract builders should ask themselves if their task it to make their target class constructor
easier to use or if their task is to write API for a factory of their target class.

Chapter 2 refers to item64.