Technical-DCache EvilCodingPractices

This is a list of things that in my opinion should be avoided in future code and cleaned up in the existing dCache code base.

• Exceptions for non-exceptional situations. Far too often, exceptions are used for quite normal situations. E.g. checking whether a file exists throws a CacheException if it doesn't exist, even though both outcomes are perfectly fine outcomes of the check.

• Avoid early catching. Littering try-catch blocks all over the code completely destroys the point of using exceptions for error-handling: The point of using exceptions is that the normal flow is apparent in the source code and not disturbed by error handling code. Using try-catch blocks around individual method calls is no better than using return codes and checking them after every call. Unless a problem can actually be handled, exceptions should simply be propagated. Use as large try-catch blocks as possible.

• Do not throw Exception. Throwing Exception, rather than a subclass, does not carry any useful information about the type of error and forces the caller to catch Exception rather than subclasses, thus forcing the caller to also catch exception that it may not reasonable be able to handle. E.g. a TimeoutException may be resolvable by retrying later, however a NullPointerException should probably be propagated. Forcing the caller to catch Exception also forces the caller to catch unexpected exceptions.

• Continuing the previous point: Avoid catching Exception, unless you are convinced that you handle all exceptional cases correctly, i.e., even RuntimeException.

• Sun provides coding style guide lines for Java, that almost all Java projects follow. For some reason, this is not the case in dCache.

• Write high-quality JavaDoc documentation.

• Avoid ad-hoc synchronization in multi-threaded code. Use monitors (like they were intended to be used) to encapsulate the object subject to concurrent access. Do not merely misuse monitors as mutex objects.