====== Introduction ======
This is a general list of guidelines and best practices collected from a variety of sources (books, interwebs, personal experience). All of these are guidelines should be taken with a grain of salt
some very helpful sources:
* Clean Code (Robert C. Martin)
* Effective Java (Joshua Bloch)
====== General ======
* Try to minimize the number of different languages (natural and artificial) in a source file
* Always follow the "Principle of the least surprise". Make things intuitive.
* Every class/method should implement the behaviour that another programmer could reasonably expect from the object's name (and nothing more).
* generally try to make the code as expressive as possible
* place code where a reader would expect it
* Do not suppress compiler warnings unless you have a very good reason
* document the reason
* suppress the warning as locally as possible
* Code Duplication is bad. DRY (Dont repeat yourself).
* although there are exceptions : Sometimes repeated code is preferable to creating dependencies between otherwise independent modules.
* Be **Consistent**
* do all similar things in the same way
* choose conventions and continue to follow them
* have only one name for one concept
* increase the expressiveness of calculations by using explanatory variables
* break calculations up by introducing intermediate variables with meaningful names
* prefer polymorphism to if/else or switch/case
* several similar switch/case statements are a good indicator that something is wrong with your code
* No magic numbers
* use well-named constants
* (does not apply only to numbers)
* Using float or double to represent currency is criminal
* use BigDecimal
* Ecapsulate Conditionals
* extract functions that explain the intent of the conditional
* Avoid negative conditionals
* they are much harder to understand
* Keep configurable data at high levels
* expose it as an argument to lower-level functions
* Prefer enums to constants
* Never write any code you dont need **right now**
* [[http://en.wikipedia.org/wiki/You_ain%27t_gonna_need_it|YAGNI]]
====== Naming ======
Names should
* be descriptive
* be unambiguos
* have appropriate length
* length of a name should be relative to the scope of the variable: the longer the scope the more precise the name should be
* as a rule: explanatory value outweighs length
* follow conventions (eg. of the domain)
* use standard nomenclature where possible
* be at the appropriate level of abstraction
* dont pick names that communicate implementation
* avoid encodings
* dont encode type information or visibility in variable names ( m_Name , s_Url)
* it is redundant, distracting and useless (the IDE does it better)
* dont't call an interface **I**Something (look at core java. List,Set,Map not IList,ISet,IMap)
====== Comments ======
Every time you write a comment you should feel **guilty**. This may sound radical but keep reading. If you write a comment you are basically admitting that you need to document a hidden concept or intention that you did not manage to **express within your code**. When you start writing a comment stop for a second and think hard if there exists a better way to express yourself within the programming language. This will not always be possible but here are a few points to consider:
* Comments need to be maintained separately. They grow stale or get separated from the code they were intended to document. They become inappropriate.
* Inappropriate comments can be distracting or even harmful
* They lead the programmer along the wrong way of thought
* Many inappropriate comments started out as being well intended, then the code evolved and the comment did not evolve with it
* Redundant comments are bad. Reiterating what your code already says is clutter at best.
* If a part of your program is so complicated that you feel the need to write a long comment then it is time to STOP. Refactor and simplify your code.
* Commented out code needs to be deleted. Always. (SVN will remember)
* Write comments **only** for things your code can not say for itself
* First try improving the naming of your variables, create adequately named helper methods or introduce clarifying variables
* Dont explain what code does (this MUST be obvious from the code, otherwise refactor until it is). Explain WHY you do it.
* If you write a comment make sure it is the best comment you can write
* use correct grammar and punctuation
* dont state the obvious
* be brief
===== API Documentation (Javadoc) =====
Public APIs should be well documented. But the above guidelines should be kept in mind. A comment like
public interface Person {
/**
* Get the birthday of the associated person.
*
* @return the birthday of the Person as Date.
*/
public Date getBirthday();
....
}
adds NO INFORMATION to the API. It does however add unnecessary clutter.
What should be documented in a Javadoc API:
* things that are **not obvious** to the user of the API
* e.g. specific details of the contract
* requirements for the input variables
* (eg. "x must be >=0" )
* the conditions under which checked and unchecked Exceptions are thrown
* eg. "@throws IllegalArgumentException if x<0"
* the state an object will be left in when an Exception is thrown
* side effects of a method (if any)
* eg. the method modifies input parameters
* eg. the method changes the state of the object (unless this is obvious)
* runtime complexity
* thread safety
* if a class is intended for inheritance (most classes really aren't)
===== Example =====
A trivial example: Instead of doing
...
if (record.getDate().before(MIN_DATE) ) { // remove if too old
remove(record);
}
do
...
removeIfTooOld(record);
....
private void removeIfTooOld(Record r) {
if (r.getDate().before(MIN_DATE) {
remove(r);
}
}
your intention is now encoded in the language itself, a comment is no longer required.
====== Classes / Interfaces ======
* Helpers/utility classes are candidates for inner classes
* Classes that are not utilities of some other class should not be scoped inside another class
* Keep all Interfaces as small as possible
* hide as much implementation detail as possible (without being overly constraining)
* make stuff private unless you have a good reason to do otherwise
* don't create lots of package/protected methods and variables (they are implementation details and other classes WILL try to depend on them)
* Possible Exception: Unit Tests (hotly debated)
* Base classes should know nothing about their derivatives
* otherwise the base class depends on the subclass (which is generally not what you want)
* Avoid artificial coupling
* things that don't depend upon each other should not be coupled
* take the time to figure out where things are supposed to be declared
* Avoid [[http://sourcemaking.com/refactoring/feature-envy|Feature Envy]]
* Avoid transitive navigation
* myObject.getB().getC().doSomething() makes it difficult to change the design. The architecture becomes rigid.
* it would be better if myObject offered the service we need: myObject.doSomething()
* Objects should primarily interact via [[http://en.wikipedia.org/wiki/Message_passing | message passing ]]
* most of your code should ask objects to DO something
* if you spend a lot of time asking objects about their internal state you are probably doing something wrong
===== Inheritance =====
* Design classes for inheritance or explicitly forbid inheritance (private constructor, final class)
* If you override a method ALWAYS declare @Override
* Classes designed for inhertiance need to document their implementation (to a certain extent)
* Protected methods are effectively part of the public interface
* Subclasses can depend on them and will break if they change in the future
* Classes designed for inheritance should never call their own public/protected methods
* a Subclass might override them and break the class invariant of the base class
* ESPECIALLY not in constructors (the subclass which overrides the method will not be initialized yet --> chaos)
* Composition is often preferable to inheritance
* Interface inheritance is fine though
====== Methods ======
* methods should never contain anything that comes as a surprise to the reader
* a method should do exactly one thing
* each method should be simple and as short as possible
* avoid control flows that are hard to follow
* avoid deeply nested constructs
* Pick clear, descriptive names
* the method name should give the reader a good idea of what the method does
* if you have to look at the implementation to find out what the method does, then pick a better name
* if you are tempted to put an AND in a method name, it may be a good idea to split the method
* Never return null when you could return an empty List (Set, Map, etc.)
* this avoids many useless if(x!=null) checks (and NullPointerExceptions)
* ESPECIALLY don't do this for performance reasons (use Collections.emptyList() )
* class methods that are used together should be close together
* order methods in a class so they can be read "top down: first a public method, then all private methods used by the public method
* try to stay at the same level of abstraction
* separate higher-level concepts from detailed concepts
* within a method don't mix levels of abstraction. It is deeply confusing.
* This also applies to Exceptions: a high-level method should NOT need to deal with low-level Exceptions
* prefer nonstatic methods
* only make functions static if they dont need an instance (duh) and you are sure you will never want this method to behave polymorphically
* when in doubt make the function nonstatic
===== Arguments =====
* Functions should have a small number of arguments (No argument is best, 3 should be max)
* Output arguments are counterintuitive. Readers expect arguments to be INPUTS. Dont confuse them.
* Boolean arguments are bad
* especially in public APIs
* a boolean argument is good indicator that a method does more than one thing and should be split
* a call like processUsers(true);
is not very helpful. Better: processAdminUsers();
* Methods that are never called are dead code and should be discarded
* unless they are part of a public API
====== Testing ======
* Only test things that are part of the class contract
* eg. on a method that returns Collection don't rely on the order of the elements during testing
* otherwise you hardcode a condition that is not part of the class contract
* Test everything that could possibly break
* dont skip trivial tests
* Tests also have documentary value
* at the very least they document how you **think** a class should behave
* Tests should be precise
* Always test boundary conditions
* Test exhaustively near bugs
* maybe a second/dependent bug is hiding nearby
* Tests should be fast
* otherwise people will not run them
* choose minimal examples that are sufficient to prove that a method behaves correctly
* Don't write several tests for the same thing
* Don't test more than one thing per @Test
* some people advocate only putting a single assert statement in every test method
* Make tests self-documenting
* descriptive names
* clear assert error messages
* develop a "testing language" on top of junit
* Don't accept failing unittests. ever.
* if a test fails fix it
* if a test is obsolete delete it
* if a test is not ready @Ignore it until it is ready
* If you ignore a unittest, document why you ignore it: @Ignore("Test case not ready")
* Keep the unittests healthy
* if you can't rely on the tests they become useless
====== Exception Handling ======
Here are a few guidelines
* Use Exceptions only for exceptional cases
* not for normal flow-control (eg. to terminate a loop)
* Before throwing an Exception think how the user of the API can/will react to it
* Use **checked Exceptions** when
* the user of your API can realistically be expected to recover (or at least do something useful after the exception has happened)
* you want the user of the API to be aware of an exceptional situation (a checked Exception forces him to handle it)
* Use **unchecked Exceptions**
* to indicate programming errors (IllegalArgumentException, IllegalStateException, NullPointerException)
* to abort in unrecoverable situations
* Provide meaningful error messages with your Exceptions
* describe exactly what has happened and why it was bad. This makes your code far easier to debug.
* Use Exceptions at the right level of abstraction
* don't force high-level methods to deal with low-level Exceptions
* employ Exception translation and Exception chaining
* Don't declare to throw multiple Exceptions unless they actually make a difference to the caller
* if they all indicate the same thing, translate the Exception into a more appropriate one
* Try to re-use existing Exceptions
* Document the conditions under which an Exception is thrown
* for checked and unchecked Exceptions ( "@throws IllegalStateException if initialize() has not been called" )
* also document if an object is left in an illegal state after an Exception
* Do not "swallow Exceptions"
* especially not when you catch something as general as "Exception"
* the least you can do is to write a log message
* If there is a really good reason for an empty catch block
* catch the most specific Exception you can
* document why the catch block is empty