====== 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