|
|
| Not logged in|Log in |
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 returnnull from a method, consider to return a
SecurityManager security = System.getSecurityManager();
if (security != null) {
//...
}
If we used null object design pattern, the code could have been simplified as follows:
// we also add interface SecurityManager
public class NullSecurityManager implements SecurityManager {
// 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) {
// security.checkRead(name);
//}
security.checkRead(name);
}
//...
}
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:
public enum Makes {
MAZDA(1),
SKODA(2), // Škoda is a Czech car
JAGUAR(3);
public final int n;
Makes(int n) {
this.n = n;
}
}
Favor composition over inheritance
A recommended way to design the relationships between classes is to use relationships between objects described. For example, for classesComputer and
Processor we will use composition because computers contain processors (relationship "has a"). For ElectricalAppliance and Computer
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 injava.util.concurrent. E.g. producer-consumer can be implemented using
BlockingQueue. Furthermore, there usually a better alternative to
synchronized collections. For example, instead of using synchronized map
(Collections.synchronizedMap(...)), one should consider
ConcurrentHashMap, 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 Lock and ReentrantLock.
It is more intuitive and more flexible than synchronized.
The following code is from the spec:
class X {
private final Lock lock = new ReentrantLock();
public void m() {
lock.lock(); // block until condition holds
try {
// ... method body
} finally {
lock.unlock()
}
}
}
Both synchronized and Lock enable exclusive locking. However,
Lock offers more. For example non-blocking locking (tryLock())
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:
public class Singleton {
private static Singleton instance = new Singleton();
private Singleton() { }
public static Singleton getInstance() {
return instance;
}
...
}
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:
public class X {
void sendMessage(String content) { ... }
void sendMessage(List<String> content) { ... }
}
km:
Zdenek, the idea is to make the intent clear in the "name" of method, e.g.:
public class X {
void sendMessage(String content) { ... }
void sendConcatenatedMessage(List<String> content) { ... }
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.
One more argument to support my view to overloading: imagine that you know the sendMessage(List
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:
class Math {
double determineCircleArea(double radius) { ... }
double determineCircleArea2(double diameter) { ... }
}
Does anybody know when overloading appeared the first time in a programming language? Could anybody point at any original paper concerning with overloading?
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:
class Parent {
void m(int x, double d) {
System.out.println("int, double");
}
}
class Child extends Parent {
void m(double d, int x) {
System.out.println("double, int");
}
}
BTW, would not be nice to have annotation @Overload here?
km:
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".
Method overriding
Should not be mentioned here that you can override only a method that is visible? Example:
package first;
public class Parent {
protected void m() { }
}
package second;
import first.Parent;
public class Child extends Parent {
protected void m() { } // this is not overriding
}
km:
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?
Zdenek: I apologize, I really do not have any clue who added 'protected' to my code :o). Of course, it should be:
package first;
public class Parent {
void m() { }
}
package second;
import first.Parent;
public class Child extends Parent {
void m() { } // this is not overriding
}
Immutable classes
Maybe one could mention that even final fields can be modified by reflection:
import java.lang.reflect.Field;
final class Final {
private final int X = (int) (Math.random() * 42);
void printX() {
System.out.println(X);
}
}
public class Main {
public static void main(String args) throws Exception {
Final p = new Final();
p.printX();
Field f = Final.class.getDeclaredField("X");
f.setAccessible(true);
f.setInt(p, 42);
p.printX();
}
}
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:
openFile();
if (error) { ... }
readFile();
if (error) { ... }
closeFile();
if (error) { ... }
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:
try {
openFile();
} catch (...) { ... }
try {
readFile();
} catch (...) { ... }
try {
closeFile();
} catch (...) { ... }
One of the main advantages of exceptions, processing of errors in one place, is not used here. One should prefer the following style:
try {
openFile();
readFile();
closeFile();
} catch (...) {
...
} catch (...) {
...
} catch (...) {
...
}
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 Math was implemented as a class with private constructor, we could have used it as a method parameter:
interface Math { ... }
class MathImpl implements Math {
private static final MathImpl instance = new MathImpl();
private MathImpl() { }
public static Math getInstance() {
return instance;
}
...
}
class Calculus {
public void figureOutLengthOfEllipse(Math m, double a, double b) { ... }
}
Also, we can permit more implementations and let the user select one, e.g. by a system property:
public abstract class Math {
private static Math instance;
static {
try {
String className = System.getProperty("mathimpl");
Class<?> clazz = Class.forName(className);
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
instance = (Math) constructor.newInstance();
} catch (Exception e) {
e.printStackTrace();
}
}
Math() { }
public static Math getInstance() {
return instance;
}
}
class MathImpl1 extends Math {
private MathImpl1() { }
//...
}
class MathImpl2 extends Math {
private MathImpl2() { }
//...
}
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:
public interface MathInt { ... }
// Math has package access
abstract class Math implements MathInt { ... }
public class MathFactory {
public static MathInt createInstance() {
return Math.getInstance();
}
}
New Strings
Let us investigate the question "Does it make sense to call the constructor
String() explicitly?". That is, String s2 = new String(s1);.
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:
public final class String
implements java.io.Serializable, Comparable, CharSequence {
/** 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;
...
}
So, a string is represented by an array of chars. Let's continue with the substring method:
public String substring(int beginIndex, int endIndex) {
if (beginIndex < 0) {
throw new StringIndexOutOfBoundsException(beginIndex);
}
if (endIndex > count) {
throw new StringIndexOutOfBoundsException(endIndex);
}
if (beginIndex > endIndex) {
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) {
this.value = value;
this.offset = offset;
this.count = count;
}
So, when we call substring() on a string, a new string will share the
array of chars with the original string. For example:
String s1 = "abcdef";
String s2 = s1.substring(2, 4); // s2 will be "cd"
There will be only one array of chars ('a', 'b', 'c', 'd', 'e', 'f').
So, both s1.value and s2.value will have the same value. Further:
s1.offset will be 0, s1.count 6,
s2.offset 2, and s2.count 2.
Further, let's have a look at the constructor:
public String(String original) {
int size = original.count;
char originalValue = original.value;
char v;
if (originalValue.length > size) {
// 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 {
// 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;
}
The constructor distinguishes two cases:
1) In the string parameter 'original', count equals value.length, 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.
count is less than value.length. 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 :) |
![[RSS]](http://wiki.glassfish.java.net/images/xml.png "Aggregate the RSS feed")