api

TODO

spi

TODO

core

TODO

docbkx

TODO

examples

TODO

build

TODO

env

TODO

Make sure the bug have not been reported earlier

TODO

Make it easy to reproduce the bug

TODO

TODO

Do not have development discussions in the bug tracker

TODO

Clearly separate fact from speculation

TODO

Class names should be nouns

Try keep class names simple and descriptive.

Exception class names should end with "Exception"

Correct.

public class ValidationException extends Exception ..

Incorrect.

public class ValidationError extends Exception ..
public class PasswordInvalid extends Exception ..

Treat abbreviations as words naming every programming element

Internal package name

Internal implementation packages should be indicated as internal by including the 'internal' immediately following the project name.

Correct.

org.deephacks.<projectname>.internal.core.admin
org.deephacks.<projectname>.internal.cache 

Incorrect.

org.deephacks.internal.<projectname>.cache
org.deephacks.<projectname>.admin.internal

Public static final fields have ALL_CAPS_WITH_UNDERSCORES

Never uses tabs in text files

Indentation should be 4 spaces for Java and 2 spaces for XML.

Use NetBeans Lookup Library

The NetBeans lookup library is intended to solve how different service classes register to the system and how other parts of the system can look them up.

Using org.openide.util.Lookup is great a great way for decoupling interactions between classes acting as services.

Use the Eclipse Code Templates

Use TODO Comments

This makes spotting ongoing work easy.

Do not use @version and @author tags

Versions are tracked by Maven and the version control system. Contributors are credited in the version control system, on project website, high level documentation and through social community meritocracy.

Methods should be small and focused, preferably with a single responsibility. If a method exceeds around 40 lines of code, consider breaking it into multiple methods.

Javadoc

Every class should have javadoc explaining its purpose and responsibilities. If you cannot think of a reasonable purpose, why create a new class? Every non-trivial method should have comments clearly explaining what it does, as well as arguments, return values and exceptions. Spend time writing comments that make code more informative. Do not include obvious documentation, comments should help understanding and readability, not prevent it.

This is particularly important for interfaces that is part of the public API and SPI. Packages should consider having an package-info.java overview explaining its usage and relevant design decisions. Please read Sun's Requirements for Writing Java API Specifications for guidance.

Be Consistent

Take a few minutes to look at code targeted for change and determine its style, then follow it.

Do not expose more than needed

Classes should be package-private by default. Only make classes and methods public if absolutely necessary. Convenience methods does not belong in the public API and SPI. When in doubt, keep it out. Restrict class interaction in order to minimize coupling among classes.

This is related to the "Tell, Don't Ask" and "Law of Demeter" principle, which increase flexibility for future design changes.

Leave room for code and data formats grow, unwise early choices can lock designs promising backward compatibility.

Always implement equals, hashcode and toString.

This make logging, collection handling and test assertions easier.

Organize constants

Do not declare constants in arbitrary classes. Keep constants in one Constants file per package and use the Constants from other classes where needed. This give a quicker and organized overview of constants.

package org.deephacks.openconfig.spi;

final class Constants {
    static final String EXAMPLE_A = ..;
    static final String EXAMPLE_B = .. ;
}

package org.deephacks.openconfig.internal.core;

final class Constants {
    static final String EXAMPLE_A = ..;
    static final String EXAMPLE_B = .. ;
}

The API are accessible for public usage by external clients.

The API have specifications that clearly describe intent and behaviour to clients. These specifications should be simple and written from perspective of clients unfamiliar with API elements.

The API keep clients loosely coupled and should therefore preferably be self-contained without forcing external third party library dependencies on clients. Classes and methods are public only if absolutely necessary. When in doubt, keep it out. This increase flexibility for evolution in the future.

The API is modular, separating its concerns cleanly and functionality is orthogonal.

The API always maintain a clear distinction between public and internal modules, in code and packaged archives. The API never have dependencies on internal programming elements or libraries.

The API is maintained and supported, should be stable and preferably evolve in a backward and forward compatible way in order to not break clients.

The API should be separate and not depend on the SPI since they are intended for different clients and evolve on different compatibility premises. The SPI is expected to have some interaction with the API and can be allowed to use it to avoid code duplication.

Don't catch Exception and Throwable

It is almost always wrong to catch RuntimeException, Exception and Throwable because it means error handling can catch exceptions that was never expected to get caught. The compiler will also not help realizing that there was new exceptions introduced after an library upgrade that may need to be handled differently.

The only place where this can be acceptable is in top level modules where exceptions possibly want to be hidden from UI. Bootstrap code also sometimes force aggressive catch behaviour to prevent processes from crashing.

Alternative ways of not swallowing exceptions depends on context.

- If the catch block can handle the exception in the catch clause, correct the error and let processing proceed or retry.

- Throw the exception directly to the caller if the exception is part of the API used by the caller.

- If the exception is not part of the API, convert it into an API-exception and throw it. Be careful not to conceal the source of error. Wrapping the original exception object is not always appropriate, because it can cause ClassNotFoundException in the client code if communication occur between unrelated class loaders. Copy the stack trace if suspecting these failures to occur.

- Throw RuntimeException including the original exception (or stack trace). This is last resort. The intention is to force propagation through the call stack to top level module and crash.

Don't log exceptions if throwing exception to caller.

See logging principles.

Exceptions shall carry data.

Use checked exceptions only for recoverable conditions

Re-throwing exceptions are a common outcome in error situations. Always make exceptions unchecked if only a minority of callers are likely to able to recover from the exception.

An end-user posting a form from a web page may cause a data input validation error deep in business code. This error is a good candidate for a checked exception. Otherwise there is a risk that the exception will slip through and bubble up to general UI error handling code and being concealed from the end-user.

But keep in mind that checked exceptions may inappropriately expose and couple higher layers to lower layer implementation details if not propagated properly through the call stack. It may be confusing for external clients to handle exceptions that is not part of the public API. This can be avoided by converting such exceptions to API exceptions.

Also keep in mind that adding new checked exceptions to method signature throws clauses (in new API and SPI versions) breaks client code.

Manage temporary conditions and failures gracefully

Some catch blocks can recover from a temporary failure conditions without causing known side-effects, such as retriable timeout failures. A temporary failure can escalate to an error situation if happening too often. Implementing back-off in and retry mechanisms is appropriate in these situations.

Do not intrude on the runtime environment

Do not make assumptions of the runtime environment. If a thread is needed to perform periodical housekeeping, it should *not* be silently spawned. External clients should be informed through API specification and under which circumstances (and how often) the thread is needed. Clients will then provide it when appropriate on behalf on the runtime environment.

Be rigorous about releasing resources

Inspect resource and failure handling code rigorously to make sure that catch and finally blocks release resources properly for all request outcomes.

Always fail fast

Avoid slow responses (and unresponsiveness) from external network communication by always setting low connection timeouts if possible. This will avoid the risk of putting the runtime environment in a unresponsive state (it "hangs") caused by shared resources eventually being consumed and exhausted. Thread.wait() without a timeout should be avoided.

Limit memory consumption

Do not consume more memory than necessary. Caches shall only consume a limited amount of memory. Java weak references or cache sizes configurable by clients can be used for this purpose.

Alarms

Alarms are used in situations where the application have serious problems, possibly needing manual corrective actions. Alarms may be a motivation for sending a distress signal to a human administrator (allegedly sleeping at home), so use alarms with extreme caution and care.

Failures can roughly be divided into two categories: temporary and permanent. Parts of the application may still be functional under any of these situations.

- Temporary failures are recoverable failures, such as sudden request disruption, inability to service certain requests, overload or temporary network failures. These kind of failures may escalate into alarming ERROR logs if occurring too frequently (a decision taken by application code).

- Permanent failures are critical application-internal problems, making the application completely unable to handle some or all requests. The application cannot spontanously recover from this situation by itself and is likely to need human administrator assistance, possibly performing disaster recovery measures, re-installation or removal/alteration of persistent data. Permanent failures raises FATAL alarm logs immediately.

Alarms are technically ERROR (temporary) and FATAL (permanent) log messages in a predefined format. It is *essential* that these messages can be parsed confidently by external supervising health-check processes, monitoring occurrence of these alarm messages in log files. Alarm message formats are therefore considered a public and crucial part of either the API or SPI, ruled by even *harder* evolving compatibility requirements. Supervising health-check processes *must* not accidentally let alarm messages pass unnoticed after an upgrade.

The external supervising process is responsible for deciding when and how to take appropriate action, and in what situations. The supervisor may decide to send a informative email describing the problem. Or as *absolute* last resort send a distress signal using SMS (or other direct communication channel) demanding immediate action from a human administrator.

All alarm messages must be throttled by application code in order to not add overhead to a potentially hysteric and overloaded runtime environment caused by enduring problems. Remember to fail fast, stepping back (rejecting immediately) doing only "occascional" retries. Alarm messages must only be logged *again* when doing these "occascional" retries to remind a potentially supervising health-check processes that the problem is still hurting end-user service. Be careful though, remember that this is a redundant repetition of an already convoyed alarm (do not spam this message), it may reach a human administrator mobile phone.

There are plans to provide a general purpose library for alarm handling.

TODO: alarm message format

Slf4j is used for logging

There is no need for guarding log statements (isDebugEnabled) when using pattern substitution.

log.debug("Found {} records matching filter: '{}'", records, filter); 

Never use Exception.printStackTrace

Obviously, nobody will monitor System.out on a production system.

Use concise and descriptive messages

Always include relevant contextual information in log messages. Include name and value of information needed to diagnose faults. If possible, make messages easy to parse by utilities such as grep, awk and sed.

Rarely log exceptions

Logging exceptions before (re) throwing them further to the caller will almost certainly cause redundant message output since caller, container or bootstrap code probably will log them as well (or worse, a combination) in the same log file. It is usually enough to only (re) throw the exception pointing to the error source.

Exceptions should never be logged on levels higher than DEBUG.

It may be appropriate to log exceptions in the following situations:

- The catch block successfully recovered from the exception, without known side-effects. Log the exception on DEBUG and consider log a (separate) WARN message. If this is a symptom that eventually may escalate into an alarm, log a WARN message using the alarm format of the failure.

Silence is golden. Remember that there are probably many other libraries doing logging simultaneously into same log file. When a program has nothing surprising to say, it should say nothing. Choose log levels carefully and do not unnecessarily (or accidentally) spam logs and exceptions into log files, possibly causing an log data avalanche headache.

Message duplication is to be avoided and make sure that no side-effects are cause by logging, such as NullPointerExceptions.

Logger names

Loggers should have same name as the class.

INFO

DEBUG

Elaborate information regarding internal life-cycle events for different elements of the design. Enabling this level should not make logs grow proportionally with number of requests (do not spontaneous log what the application is doing on behalf of a specific user using DEBUG). Events are, for example, internal startup and shutdown messages, lazy cache-refresh events, internal periodic timeouts etc.

This level is also used to log exceptions from temporary and permanent failures.

TRACE

Mostly used for tracing request calls that comes in and out from API, SPI and external systems (such as file storage, network etc), allowing for deep probing of data flows. Can also be used for tracing complex calculations/algorithms in event of potential malfunctioning.

This information can make diagnosing faults in log files from untrusted systems easier.

WARN

Something unexpected happened that may have consequences for the user. The error is partial, temporary or recoverable, but may turn into a ERROR alarm situation. Application is likely to continue service requests for the time being.

ERROR

Indicate escalation of partial, temporary or recoverable failures.

Log the error code and contextual information in format specified by type of error. Also log the exception separately on DEBUG level.

See alarms for more information.

FATAL

The application have seemingly permanent and critical problems internally.

Log the error code and contextual information in format specified by type of error. Also log the exception separately on DEBUG level.

See alarms for more information.

Allow external clients to adapt to their current runtime environment

Clients should be able to adapt configuration sensibly to all their different test, stage or production runtime environments(s).

Configuration values are enforced in the following order.

Clients should be allowed to configure what SPI service providers are used at runtime using java.util.ServiceLoader.

Network resource addresses (databases for example) cannot usually be assigned a sensible default value because the runtime environment is unknown.

JUnit is used for unit testing

TODO

Mockito is used for mocking

TODO

Test public behaviour, not implementation details

Think black-box. Classes should first and foremost be tested through publicly available methods. Only conservatively test private methods, since it will increase the maintenance burden and make refactoring hard.

Prefer adding tests to the API and SPI if possible. These tests also exercise the code in ways similar to real external clients.

Use mocks judiciously

Tests should assert behaviour of a well-defined unit, that is, post-conditions (and side-effects) for a given set of pre-conditions. Rarely should it assert the "dancing" path between collaborators within the implementation of that unit.

Mocks insist upon exact verification of interaction with it, which couple tests tightly to implementation details of the test subject. Tests written this way become "brittle”, they fail when unrelated changes are made to the code.

Testing internal cache behavior is good example where mocking is appropriate, since the test cannot assert whether the cache hit or missed simply by looking at the return value.

Use org.openide.util.test.MockLookup

MockLookup makes it easy to override the default implementations of a given service using org.openide.util.Lookup, without any change to the objects that use the service.

This help writing classes that does not compromise encapsulation and remove the need for adding methods for setting mock services that are only used by tests.

Do not duplicate fixture setup and code between tests

It may seem obvious, but this easily happens when writing many similar tests that only vary slightly in terms of fixture and expectations.

It can be appropriate to create Test Data Builders that produce and manage fixtures to be used by test. Builders have default values for objects they create, which enable tests to specify that little variation needed and thus eliminate duplication.

Classes that serve as an interface for a separate and noticeable module in the architecture should consider providing a Fake Object of itself that can be used by its users during testing in order to isolate certain domain boundaries. A Fake Object is a very simple and lightweight version of the real class/module implementation. This can, in comparison to mocks, avoid duplication and help reduce the amount of fixture/behaviour code that needs to be written between tests.

Modules can provide Test Data Builder and Fake Object of their public classes through a test library, rather than putting a mock-burden on external clients. Forcing external clients to mock interfaces may result in too many (possibly incorrect and deviating) assumptions on implementation details.

Balance this technique with good intuition and only apply it where appropriate. Keep in mind that creating too many Fake Objects can turn into a maintenance burden in itself.

JUnit Rules can be an excellent way for setting up cross-cutting test fixtures. Rules intercept test execution with pre and post hooks that can be used to inspect the method that is to be executed. This means that it is possible to, for example, decorate test methods with annotations to setup behaviour fixtures. This is an example on how this technique could be utilized.

public class SomeTest {

  @Rule
  public FixtureRules rule = new FixtureRules();

  @Test
  @CustomerService(MALFUNCTIONING_FAKE)
  @Concurrency(10)
  public void myFirstTestMethod() throws {
    ..
  }

  @Test
  @Transactional(ROLLBACK)
  public void mySecondTestMethod() throws {
    ..
  }
  ...
}

Tests should be focused, robust and self-contained.

Clearly document purpose of each test and do not assert post-conditions or side-effects that are not related to its purpose.

Tests should be independent and never rely on other tests for success. Tests do not depend on the order in which they are executed. Imaging what this would be like, doing casual development in Eclipse and trying to understand what behaviour tests are intended verify.

Keep this in mind, especially when writing tests that involve persistent data. Write tests that does not rely on clean up code to execute after the test is finished. Test can be interrupted in the middle of execution which accidentally can leave data lying around. Ensuring that pre-conditions are satisfied *before* execution tests tends to be more robust.

Or even better, not making too many assumptions of current state of test subject. For example, adding two persistent entities during a test, assert that those *specific* entities exist (using lenient ordering) instead of asserting size == 2. Also making sure that primary and composite keys are generated uniquely by the test to avoid EntityExistsException.

Test failure scenarios

It is common to see test suites that exercise applications under normal conditions, ignoring tests that analyse how the application behave under influence uncertainty and unforeseen failures.

Use Unitils's assertion utilities

Unitils is a library that have assertion utilities that greatly can reduce the amount of assertion code that needs to be written.

Automation

All tests should be automated and build and execute successfully on any machine that conform to development environment requirements.

Ask Question the Smart Way

There is nothing wrong with having strong opinions, but do not express them along with conclusions with so much certainty that others feel pointless to disagree.

Take time to phrase critique and disagreement in a constructive and relevant way. Show the ability to adopt the perspective of another's work.

Be collaborative and helpful toward fellow contributors. We are all one.

Avoid private and off-topic discussions in public communication channels.

Public communication channels have zero-tolerance policies toward rude or insulting behavior.