GlassFish Wiki : CommentsOnEffectiveJava1Companion

Comments on Effective Java - 1 Companion

This is the Wiki topic that is devoted to your comments on Effective Java - 1 Companion. All you need is a GFWiki user name and password. At the moment, it is (unfortunately) different from your java.net ID. Please comment based on the anchors in above document.

Use null object design pattern

If you return <code>null</code> from a method, consider to return a <it>null object</it>.
For example, Java contains the following code in many places:

<pre>
SecurityManager security = System.getSecurityManager();
if (security != null) {
//...
}
</pre>

If we used null object design pattern, the code could have been simplified as follows:

<pre>
// we also add interface SecurityManager
public class NullSecurityManager implements SecurityManager

Unknown macro: { // all methods of SecurityManager will be empty public void checkRead(String name) { }

//...
}

public class FileInputStream extends InputStream {
//...
public FileInputStream(File file) throws FileNotFoundException {
//...
SecurityManager security = System.getSecurityManager();
//if (security != null)

Unknown macro: { // security.checkRead(name); //}

security.checkRead(name);
}
//...
}
</pre>

Do not rely on enum's ordinals

Do not use enum's ordinals e.g. for indexing in array. If you need to assign an integer to each enum constant, you can declare a field:

<pre>
public enum Makes {
MAZDA(1),
SKODA(2), // Škoda is a Czech car
JAGUAR(3);

public final int n;

Makes(int n)

Unknown macro: { this.n = n; }

}
</pre>

Favor composition over inheritance

A recommended way to design the relationships between classes is to use relationships between objects described. For example, for classes <code>Computer</code> and
<code>Processor</code> we will use composition because computers contain processors (relationship "has a"). For <code>ElectricalAppliance</code> and <code>Computer</code>
we will use inheritance because computers are electrical appliances (relationship "is a").

The decorator design pattern should not be mentioned here (or at least not the way it is). The purpose of the decorator is to change the behaviour of another object (decoratee). And this is not the case here.

Concerning the question whether to make classes final or not, it would depend on the project. For open source libraries and frameworks, I personally think that if we do not have very good reasons (e.g. security) to make a class final, it should not be final. If we do not want somebody to extend the class, we can write it to the documentation. But why to restrict users of our classes if we do not have to? For company projects, it could be reasonable to make classes final because we can keep colleagues from extending them. So, this supports the usage we want to.

Forget about synchronized and use java.util.concurrent

For common tasks, such as producer-consumer, there are already classes in
<code>java.util.concurrent</code>. E.g. producer-consumer can be implemented using
<code>BlockingQueue</code>. Furthermore, there usually a better alternative to
synchronized collections. For example, instead of using synchronized map
(<code>Collections.synchronizedMap(...)</code>), one should consider
<code>ConcurrentHashMap</code>, which enables several threads at the same time.
For example, three threads can read and two threads can update the map without any
blocking.

If you really need locking, use <code>Lock</code> and <code>ReentrantLock</code>.
It is more intuitive and more flexible than <code>synchronized</code>.
The following code is from the spec:

<pre>
class X {
private final Lock lock = new ReentrantLock();

public void m() {
lock.lock(); // block until condition holds
try

Unknown macro: { // ... method body }

finally

Unknown macro: { lock.unlock() }

}
}
</pre>
Both <code>synchronized</code> and <code>Lock</code> enable exclusive locking. However,
<code>Lock</code> offers more. For example non-blocking locking (<code>tryLock()</code>)
and fairness (it prevents starvation).

Lazy initialization

Lazy initialization is not very useful in Java because classes are loaded just before the first use. For example:

<pre>
public class Singleton

Unknown macro: { private static Singleton instance = new Singleton(); private Singleton() { }

public static Singleton getInstance()

Unknown macro: { return instance; }

...
}
</pre>

Class Singleton is fetched at the time of first call to Singleton.getInstance(), so the initialization of 'instance' is postponed till this time. One can also say that it is lazy initialization due to dynamic class loading.

It has some limitations: e.g. if we had a class variable in Singleton, an access to this variable would have initialized 'instance'.

Method overloading

I agree that method overloading should be used cautiously. However, I do not see any motivation for the rule "don't overload methods if they take same number of parameters". Should not the rule be rather "overload methods ONLY if they do the same job"? For example:

<pre>
public class X {
void sendMessage(String content)

Unknown macro: { ... }

void sendMessage(List<String> content)

}
</pre>

km:
<pre>
Zdenek, the idea is to make the intent clear in the "name" of method, e.g.:
public class X {
void sendMessage(String content)

Unknown macro: { ... }

void sendConcatenatedMessage(List<String> content)

</pre>

Zdenek:
Kedar, if you apply this approach, you do not need method overloading at all. You could have had printInt(), printShort(), printBoolean(), etc. in PrintStream. But do you really think that this approach would have been better than overloaded methods print()? I do not, because the name of the method is descriptive enough and the programmer does not have to remember if there is printShort() or not (and in case if not what method should be used for short values). She only calls print() and the compiler finds the most suitable method.

Concerning the sendMessage() above, if the messages sent by these two methods are of same type and differ only in content (for example, both of them are JMS TextMessage), I think overloading should be preferred here. A clue can be in this case: can a recipient of the message distinguish between messages sent by sendMessage() and sendConcatenatedMessage()? If not, it means that the methods do the same job (but with different data) and thus they should have the same name. Different names should be used only if they send different messages. For example, if sendMessage() sends TextMessage and sendConcatenatedMessage() sends ObjectMessage.
<hr>

One more argument to support my view to overloading: imagine that you know the sendMessage(List<String> content) method (you read the javadoc and used it several times). And now you want to send a message containing a single string. The context help will offer the sendMessage(String content) method. What will you do? I will expect that the method does the same as sendMessage(List<String> content) and probably call it without reading the javadoc.
<hr>

Now it seems to me that the right rule should be: We should use overload methods if and only if they do the same jobs. It is logical to use the same name for methods that produce the same result and it is misleading to use different names for the same actions. The only exception allowed from this rule is when overloading is not possible. For example, when two methods do the same and accept the arguments of same types:

<pre>
class Math {
double determineCircleArea(double radius)

Unknown macro: { ... }

double determineCircleArea2(double diameter)

}
</pre>

Does anybody know when overloading appeared the first time in a programming language?
Could anybody point at any original paper concerning with overloading?

<hr>

From the sentence "All overloaded methods are implemented in a given class only." one could possibly think that you cannot overload a method from a superclass, which is not true:

<pre>
class Parent {
void m(int x, double d)

Unknown macro: { System.out.println("int, double"); }

}

class Child extends Parent {
void m(double d, int x)

Unknown macro: { System.out.println("double, int"); }

}
</pre>

BTW, would not be nice to have annotation @Overload here?

km:
<pre>
Zdenek, I agree with you regarding annotation. It would be nice to have such annotation.
Also, I will remove the statement that says "overloading can be done in the same class only".
</pre>

Method overriding

Should not be mentioned here that you can override only a method that is visible? Example:

<pre>
package first;

public class Parent

Unknown macro: { protected void m() { }

}
</pre>
<pre>
package second;

import first.Parent;

public class Child extends Parent

// this is not overriding
}
</pre>

km:
<pre>
Zdenek, I am confused. It is obvious that method to be overridden is to be visible.
But the example cited by you already satisfies it.
So, I wonder why is it is NOT an example of overriding?
</pre>

Zdenek:
I apologize, I really do not have any clue who added 'protected' to my code :o). Of course, it should be:
<pre>
package first;

public class Parent

Unknown macro: { void m() { }

}
</pre>
<pre>
package second;

import first.Parent;

public class Child extends Parent

// this is not overriding
}
</pre>

Immutable classes

Maybe one could mention that even final fields can be modified by reflection:

<pre>
import java.lang.reflect.Field;

final class Final {
private final int X = (int) (Math.random() * 42);
void printX()

Unknown macro: { System.out.println(X); }

}

public class Main {
public static void main(String[] args) throws Exception

Unknown macro: { Final p = new Final(); p.printX(); Field f = Final.class.getDeclaredField("X"); f.setAccessible(true); f.setInt(p, 42); p.printX(); }

}
</pre>

Item 3 of "Recipe for Writing Correct Immutable Classes": the memory model has already been reworked.

Use exceptions properly

In languages like C, where we do not have any exceptions, the code looks like as follows:

<pre>
openFile();
if (error)

Unknown macro: { ... }

readFile();
if (error)

closeFile();
if (error)

Unknown macro: { ... }

</pre>

A drawback of this approach is that we have to check if something bad happened after each call. The code of application is mixed with checking for errors in such case and we must be careful not to miss any checking.

Exceptions remove this drawback and make the programmer's life easier if they are used properly. However, the proper use is not this:

<pre>
try {
openFile();
} catch (...)

try {
readFile();
} catch (...)

Unknown macro: { ... }

try {
closeFile();
} catch (...)

</pre>

One of the main advantages of exceptions, processing of errors in one place, is not used here. One should prefer the following style:

<pre>
try {
openFile();
readFile();
closeFile();
} catch (...) {
...
} catch (...) {
...
} catch (...) {
...
}
</pre>

Singletons

Let us answer the question "Should I implement the Singleton design pattern as a class with static methods or as a class with private constructor and static field?". Although the approach "class with static methods" is correct in some cases, it has a serious drawback. The drawback is that we lose many benefits of objects and late binding. For example, we cannot pass the singleton as a parameter to a method or exchange the implementation.
If <code>Math</code> was implemented as a class with private constructor, we could have used it as a method parameter:

<pre>
interface Math

Unknown macro: { ... }

class MathImpl implements Math

Unknown macro: { private static final MathImpl instance = new MathImpl(); private MathImpl() { }

public static Math getInstance()

...
}
class Calculus {
public void figureOutLengthOfEllipse(Math m, double a, double b)

Unknown macro: { ... }

}
</pre>

Also, we can permit more implementations and let the user select one, e.g. by a system property:

<pre>
public abstract class Math {
private static Math instance;
static {
try

Unknown macro: { String className = System.getProperty("mathimpl"); Class<?> clazz = Class.forName(className); Constructor<?> constructor = clazz.getDeclaredConstructor(); constructor.setAccessible(true); instance = (Math) constructor.newInstance(); }

catch (Exception e)

Unknown macro: { e.printStackTrace(); }

}
Math() { }
public static Math getInstance()

Unknown macro: { return instance; }

}
class MathImpl1 extends Math

Unknown macro: { private MathImpl1() { }

//...
}
class MathImpl2 extends Math

Unknown macro: { private MathImpl2() { }

//...
}
</pre>

In this case, we relieved the Singleton property because other subclasses of Math can be created. If this is not desirable, we can combine this approach with the Factory method pattern:

<pre>
public interface MathInt

// Math has package access
abstract class Math implements MathInt

Unknown macro: { ... }

public class MathFactory {
public static MathInt createInstance()

Unknown macro: { return Math.getInstance(); }

}
</pre>

New Strings

Let us investigate the question "Does it make sense to call the constructor
<code>String()</code> explicitly?". That is, <code>String s2 = new String(s1);</code>.

Zdenek: This is my answer to a similar question from the alias of Java instructors:

Let's have a look at the source code of String first:

<pre>
public final class String
implements java.io.Serializable, Comparable<String>, CharSequence

Unknown macro: { /** The value is used for character storage. */ private final char value[]; /** The offset is the first index of the storage that is used. */ private final int offset; /** The count is the number of characters in the String. */ private final int count; ... }

</pre>

So, a string is represented by an array of chars. Let's continue with
the substring method:

<pre>
public String substring(int beginIndex, int endIndex) {
if (beginIndex < 0)

Unknown macro: { throw new StringIndexOutOfBoundsException(beginIndex); }

if (endIndex > count)

Unknown macro: { throw new StringIndexOutOfBoundsException(endIndex); }

if (beginIndex > endIndex)

Unknown macro: { throw new StringIndexOutOfBoundsException(endIndex - beginIndex); }

return ((beginIndex == 0) && (endIndex == count)) ? this :
new String(offset + beginIndex, endIndex - beginIndex, value);
}
// Package private constructor which shares value array for speed.
String(int offset, int count, char value[])

Unknown macro: { this.value = value; this.offset = offset; this.count = count; }

</pre>

So, when we call <code>substring()</code> on a string, a new string will share the
array of chars with the original string. For example:

<pre>
String s1 = "abcdef";
String s2 = s1.substring(2, 4); // s2 will be "cd"
</pre>

There will be only one array of chars ('a', 'b', 'c', 'd', 'e', 'f').
So, both <code>s1.value</code> and <code>s2.value</code> will have the same value. Further:
<code>s1.offset</code> will be 0, <code>s1.count</code> 6,
<code>s2.offset</code> 2, and <code>s2.count</code> 2.

Further, let's have a look at the constructor:

<pre>
public String(String original) {
int size = original.count;
char[] originalValue = original.value;
char[] v;
if (originalValue.length > size)

Unknown macro: { // The array representing the String is bigger than the new // String itself. Perhaps this constructor is being called // in order to trim the baggage, so make a copy of the array. int off = original.offset; v = Arrays.copyOfRange(originalValue, off, off+size); }

else

Unknown macro: { // The array representing the String is the same // size as the String, so no point in making a copy. v = originalValue; }

this.offset = 0;
this.count = size;
this.value = v;
}
</pre>

The constructor distinguishes two cases:
1) In the string parameter 'original', <code>count</code> equals <code>value.length</code>, i.e. all the character of the array belong to this string. Then, a new
string will share the array of chars with the original string.
2) The string parameter is a substring of some other string, i.e.
<code>count</code> is less than <code>value.length</code>. Then a new array of chars is created because if original string is not referenced anymore, we want it to be
deallocated (see the comments in the source). This also answers
the question when it makes sense to call this constructor.

ID	Anchor	Comment
0	No anchor, overall.	Looks good, useful