Java Best Practice,

(see also Code Smells)

Considered Harmful...

Declaring variables (and setting them to null) at the top of a method before they are used. Leads to dangerous reuse of variables, bloat, and is a great source of null pointer exceptions. Declare Variables at First Use

Declaring a constant for a value that only ever occurs once in code. Bloaty. [em]checkstyle complains about magic numbers if you don't - a rule to demote to info level? -- JohnTaylor - 11 Mar 2004[/em]

Monster classes. Taller Than Me

Calling nonfinal methods from constructors. (EJS rule 81. Unfortunately can't be trapped by checkstyle. Bitten me more than once this one.) -- JohnTaylor - 11 Mar 2004

Exceptions

Catching exceptions too early, and then returning boolean 'success' codes. Exceptions are more descriptive, and have to be handled. boolean flags can be so easily ignored.

Catching exceptions early, logging a message, and then continuing regardless. Don't Catch Exceptions

Logging

Excessive logging - e.g. logging entry and exit to each method. Bloats the code, and makes logs that much harder to follow. Use a debugger. Logging Best Practices

Using finally just to log exit of a method - inefficient, bloaty, and makes the previous point even more pointless - as the exit to a method will always be logged, no matter what happens in the method itself.

Debug flags in code. Dead code elimination isn't so great - it bloats the source code, and you never know when you might want to turn logging back on in an installation. The logging library(s) we've settled on allow the logging level to be altered at runtime.

Instead, Try to

Define new types, instead of passing Strings around. Java has a strong type system, so use it to structure the code. A method should only accept a string if it can validly handle (almost) all possible string values.

Avoid Final Strings for Unique Types - a slight variation which I think is even better is the JavaEnumIdiom - JohnTaylor - 25 Feb 2004

Similarly, use java.net.URL, java.net.URI, java.io.File more.

Write fewer static methods.

Avoid writing classes with more than 10 public methods. Which would you rather try to work with? - 20 classes with 10 methods each, or 1 class with 100 methods?

Close down types - don't make methods and classes public by default. Does that field really need a public getter and setter?

Even better, define public interfaces for the methods that your classes expose, to partition funtionality into related subsets Masking Interfaces, Build Interface Implementation Pairs

Only use logging to record branches taken through code, things that seem quesionable, and errors. Isn't it handy to see param values etc sometimes? If you set the logging level low enough e.g. INFO, you can always switch it off. Perhaps we need to agree on what the different logging levels should be used for. JohnTaylor - 25 Feb 2004

Remove dead code, and refactor, as you code. Refactoring should be a continual process. Yes Yes Yes. Need to have JUnit tests in place first so you can be (fairly) sure you don't break anything. JohnTaylor - 25 Feb 2004

Use asserts JohnTaylor - 25 Feb 2004 but dont overuse them - remember that they can be switched off at runtime - your program logic must not depend on their presence -- PaulHarrison - 26 Feb 2004

Documentation

Wouldn't it be great if

every package had a package.html. Even if it had just a little bit of description in it.
every class had a javadoc comment that described what it did. just a sentance.
Every method that accepts or returns a collection like Iterator, List, Hashmap, etc has a javadoc comment that clearly states the type of object to expect in the container

However..

Too many line comments in the code make it more difficult to read - avoid this sort of thing

            // If the path is not null.
            if (null != path)

that simply echos what the code says - say something in the comment that is not obvious from a simple reading of the code statement - explain why more often than how.

-- PaulHarrison - 25 Feb 2004