Java Coding Guidelines

By introducing CoreWallet coding guidelines we want to ensure that a maintainable, readable and clean code will be shipped. The guidelines must be applied by every engineer working on the trimplement CoreWallet and usage should be verified by code reviews when needed. Additionally, this document covers some rules for implementing beans, services and controllers that are following the CoreWallet standard.

Java Style

The general java coding style is based on the original coding guidelines released by Sun in 1997. This style is old but still common. Latest version can be found, but the document is not maintained any more: http://www.oracle.com/technetwork/java/codeconventions-150003.pdf

Summary

  1. Write simple, legible code - we're not trying to create art
  2. Write consistent code with consistent solutions - it's not a playground for experiments
  3. Write robust code - always assume the unhappy-case
  4. Test your code - both in unit and integration tests; make sure that each individual unit/integration test can be run by itself either from the IDE or the command line builder tool
  5. Document your code - assume you're not going to maintain your code

trimplement Java Style

This section describes which differences trimplement uses for the CoreWallet style.

Line Length

Code lines may exceed a length of 80 characters, but should not exceed a length of 240 characters. The general idea should be to have lines that can be viewed with a standard screen resolution for today (assuming at least 16xx px width).

Empty lines

Do not use more than one subsequent empty line. Write compact code that's easy to read, and does not require excessive scrolling because of numerous empty lines.

Comments

Single-line comments should start with two slashes and a space. JavaDoc comments for classes, enum and interfaces should be present and should not include the author’s name, because git-blame can be used to reliably determine the author of the latest changes. Methods should be also complemented with JavaDoc comments.

/**
 * JavaDoc style headers
 */
public class SomeClass {
    /**
     * This method serves as an example to illustrate comments' style.
     *
     * @param someParam some param of some type
     * @return 
     * @throws SomeException
     */
    public void someMethod(final SomeType someParam) throws SomeException {
        // That's the comment
        //Not like that 
    }
}

Method Header

For method header parameter lists there are two possible ways for wrapping: don’t wrap or one line per argument. General rule should be to have methods with fewer parameters in one line, and break methods with many parameters into multiple lines. See the below examples.

public Wallet getWallet(final String walletId, final Status status) throws WalletException {
    // Code goes here
}
public Wallet listWallets(
            final Date createdFrom,
            final Date createdUntil,
            final Status walletStatus,
            final String walletAccountId,
            final String subsidiaryId
    ) throws WalletException {
    // Code goes here
}

Declarations

Always use one declaration per line at maximum, except method parameter list. Declare variables final per default. Only remove final if the variable is intended to be updated, this should be rather exceptional though. Not included in this rule are getter/setter methods, which do not require final in the parameter declaration, and exceptions in catch-clause, because they do not need to be final as it's usually only logging/wrapping. Use single space between [keyword(s)] type name.

Classes

If possible make small data classes immutable, e.g. Money, CurrencyConversionInfo,…

Enums

Enum names should be 'PascalCase' even though Sonar complains about constant-naming-conventions. We don't want our Enums to SHOUT.

Statements

The above document mentions that if statements should always use braces {}. For CoreWallet, the rule is: all control statements that can be followed by a block must use braces {}.

Loops

Never use empty loops.

Switch

All switch statements must handle the default case in a proper way, i. e. throw an exception, or log a warning, and use a break; if no exception is thrown.

Lombok

Use lombok annotations to have boilerplate code generated, e.g. constructors, getter/setter methods, builders, etc. Keep in mind that the lombok builder pattern does not work nicely with inheritance.

HashCode / Equals

For classes that implement HasID or HasGenericID interface, the HasIDUtils.hashCode() / HasIDUtils.equals() should be used instead of generated hashCode/equals with many if-statements.

For other classes, HashCodeBuilder.hashCode() / EqualsBuilder.equals() should be used.

ToString

Only include relevant fields in a toString() implementation, otherwise the log will overflow with irrelevant info. Use lombok @ToString.Exclude to exclude attributes from a generated toString() method.

Deprecations

If code needs to be phased out but cannot be removed immediately, it can be annotated with the @Deprecated annotation. In that way, it has to be documented why it was deprecated and what the alternatives are.

Expressions

Elvis (?:) expressions

Since Java8 Optional.ofNullable(a).orElse(b) should be used instead of a != null ? a : b. It is typically more compact as you don't have to repeat expression a. It only works when neither expression throws a checked exception, and comes at the cost of an object instantiation.

CoreWallet Architecture/Style

This document is not only about coding formatting conventions, but also about how to implement database beans, services and controllers that are compliant with the trimplement standards/CoreWallet architecture.

Overview

A typical feature example consists of a database entity/DAO, service/job, and a controller.

Usually this leads to three different representations of the entity on DB level, service level and API level. This ensures that only relevant data is exposed and encourages best practices like converting database IDs to public IDs.

Database objects are prefixed with “Db”, like DbMyFeature. Service level objects have no pre- or suffix, like MyFeature. Controller level objects use an “Info” suffix, like MyFeatureInfo. There are two converter classes required: from database to service level and from service to API level. We never expose internal (database) IDs in API level objects. The converter classes are non-instantiable and must provide at least one static method named like toMyObject for service level and toMyObjectInfo for controller level. Normally, one also wants to provide a toMyObjectList and similar for controller level converter. Always check for null and take care of proper ID/object converting.

Sensitive Data

Use SecureString for sensitive data, to avoid e. g. accidental logging of passwords or credit card numbers.

Naming Conventions

All service interfaces are followed by “Service”. Service implementations use “ServiceImpl”. Tests use “ServiceTest”. All classes related to a service should be named in a similar way, so that all classes for a feature can be identified easily. Here are some examples:

MyFeatureService
MyFeatureServiceImpl
MyFeatureServiceTest
MyFeatureJob (if required)
MyFeatureJobTest (if required)
DbMyFeature
MyFeature
MyFeatureInfo
MyFeatureDAO
MyFeatureDAOHibernateImpl
MyFeatureApiController
MyFeatureApiAdminConroller
MyFeatureConverter (for both service/controller level)
…

For method name, be concise but clear, and provide a clear indication what the method does. In some cases, you may want to abbreviate the entity-name in the method-name, e.g. if it's very long (e.g. TermsAndConditionsConfiguration)

OK: getViewName
OK: calculateFees
OK: determineAccountBalance
OK: listWalletAccountItems
OK: createWalletAccount
OK: fetchExchangeRates
OK: isDuplicateCheckEnabled
NOK: doStuffForOtherStuff
NOK: a
NOK: whetherOrNotThisFlagIsEnabled
NOK: shouldThisCheckBeDisabledOrEnabled
NOK: cgnSThng
NOK: getTheExchangeRatesFromTheConfigurableExternalServiceProvider
NOK: willThereBeSnowyChristmasThisYear

For parameter/attribute names, be concise but clear, and do not write prose in the name:

OK: someCounter
OK: preventDuplicateCheck
OK: dbWallet
OK: originalCount
NOK: thisValueWillPreventThatTheMethodDoesTheCheck
NOK: countThatWillBeTheResultOfTheAddition
NOK: tblCgn
NOK: a1

Converter

To translate dom objects between service layers we typically use converters.

In converters we should use for simple shallow copy org.springframework.beans.BeanUtils.copyProperties(source, target)

This reduces code duplication.

Services

All services should be seen as public in the first place. Public means that the service is exported via http invoker and therefore using the Spring security (-context). There might also be pure internal services, but these cases are rare. For public services there are the following general rules:

  • The service interface must extend ExportedService to ensure default authorization level via @PreAuthorize(SecurityUtils.DEFAULT).
  • If a service interface extending ExportedService contains any read only methods, they must be annotated with @ReadOnly annotation to indicate they are read only methods.
  • The first parameter of all exported service methods must be of type ApiRequestInfo, as the interceptor will extract the security-context from there. If it is not found, an Anonymous security-context is set.
  • For all service methods one needs to carefully think about required authorization. System, admin, user, ...?
  • All services usually provide a default get/list method for the entity introduced by the service. There might be reasons not to provide a list method, but that should be the exception. More requirements for get/list will follow in section “DAO”.
  • If it is to be expected that the parameter list of a method will grow over time, it is in general a good way to have a holder object for all required parameters except the  ApiRequestInfo, which must be stand alone (because intercepted, see above). See e. g. CreateWalletRequest for an example for a request holder.
  • The methods that require object IDs as parameters should use public IDs which are represented by a String or PublicTypedID unless not possible because the referenced entity does not have a public ID.
  • The service implementation should have a @Service("service-name") annotation, so the service can be used both with XML and annotation-based configurations in products
  • Service implementation attributes should be @Autowired or @Value unless they are internal-only attributes, so the service can be used both with XML and annotation-based configurations in products

Database

General

Table names are collected in the specific constants file, like e. g. WalletConstants.DB_TABLE_MY_TABLE;. Column names and length definitions are public constants of the DB entity class, prefixed with COL_/LEN_ or COLUMN_/LENGTH_. Length can also be defined in a parameter-interface like the above mentioned WalletConstants.

All timestamps/dates/times in the database are always stored in UTC (or GMT). This is required to allow queries on the date column, and prevent ambiguous values that cannot be easily interpreted. When timezones are needed (e.g. for billing), the application must convert UTC dates to the appropriate timezone in the appropriate converter.

Database Beans

Getter/Setter

We use auto generated no-comment getter/setter for database entities using lombok annotations.

Equals/hashCode/toString

For all objects with IDs (which is true for all database objects), we use HasIDUtils.hashCode(this); for hashCode() and HasIDUtils.equals(this, obj); for equals. If the object contains a reference to another database object we use HasIDUtils.toIdString(reference) in toString.

Identifier representations

There are various requirements for a bean, but handling IDs is probably one of the most important topics. The CoreWallet provides IDs that are built of two parts: a type and the actual (long) ID. This is called a TypedID.

In general there are three different kinds of IDs which differ in their use: Internal IDs, Display IDs and Public IDs. Internal IDs are the database IDs itself, usually a long for service internal usage. Display IDs are intended to be human-readable IDs that will be shared with the customer. Public IDs are intended to be used by the API and thus anyone connecting to the CoreWallet API/services. Both display/public IDs are represented by a String. For public IDs there is also a PublicID object, which is the public variant of a TypedID, using the same type.

Identifier handling

Db*-classes should always implement HasTypedID and HasPublicTypedID interfaces if exposed to the outside. When none of the above interfaces is applicable, HasID or HasGenericID should be used.

HasTypedID requires a so-called “Typed ID type”, which is defining the type of the entity which the ID represents. Usually this type is declared by a public constant in the service level representation of the entity because it needs to be visible to the converters and other places. Furthermore the converter class must define a PublicIdUtil.

On service level, the main ID of the object will be the public ID whereas the internal ID becomes “special”. Meaning, the internal ID should only be accessible if required. Accordingly, service level objects can implement the HasInternalID interface.

Also on service level one decides to use a display ID or not. If yes, the object needs to implement the HasDisplayID interface and to statically register the typed ID type via DisplayIdConverter.registerType(MyTypedIDType);.

For cases where the object is connected to a wallet or a subsidiary, the provider-interfaces WalletProvider or SubsidiaryProvider could be used, which are also providing HasTypedID-support.

Hibernate specific

  • DB names (table, index, sequence, constraint, column, …) must not be longer than 30 characters for Oracle compatibility
  • Always define column length as statically accessible variables either in a parameter-interface (e. g. WalletConstants.java) or in the corresponding entity, so it can be used in parameter-validation in the service-implementation
  • Do not call dao.update() for entities received with findById inside a transaction
  • Use @AccessType(WalletConstants.HIBERNATE_ACCESS_PROPERTY) for primary key attribute so the ID-property can be accessed without triggering a lazy-load
  • Annotate generated id fields with the @GenericGenerator and strategy CommonConstants.DB_CUSTOM_SEQUENCE_GENERATOR to enable products to implement id sharding. See the JavaDoc of CustomSequenceStyleGenerator for some more details. Example annotation from DbWallet:
@GenericGenerator(
    name = WalletConstants.DB_GENERATOR_WALLET,
    strategy = CommonConstants.DB_CUSTOM_SEQUENCE_GENERATOR,
    parameters = {
        @Parameter(name = CustomSequenceStyleGenerator.SEQUENCE_PARAM, value = WalletConstants.DB_GENERATOR_WALLET)
    }
)
  • Use @Fetch(FetchMode.SELECT) for many-to-one relations that are lazy, so second-level-cache is used (instead of join)
  • Do not use one-to-many associations in mapping where collection-size may exceed 10 items (or it will kill performance or the whole server for large collections) - use a DAO find method with Paging which returns Page for that
  • DAO find methods that return a collection that may have more than 10 items must have a paging attribute to limit result set size
  • Use CommonConstants.ENUM_LENGTH for enums and always store them as Strings
@Column(name=COL_WALLET_STATUS, length=CommonConstants.ENUM_LENGTH, nullable=false)
@Enumerated(EnumType.STRING)
private WalletStatus walletStatus;
  • Money must always be stored with the proper user-type, and use the proper length/precision/scale for currency/amount
@Type(type=MoneyUserType.TYPE)
@Columns(columns={
    @Column(name=COL_MERCHANT_AMOUNT_CURRENCY, nullable=false, length=DbCurrencyInfo.LEN_CURRENCY_CODE),
    @Column(name=COL_MERCHANT_AMOUNT_AMOUNT, nullable=false, precision=MoneyUserType.PRECISION, scale=MoneyUserType.SCALE)
})
private Money merchantAmount;

DAOs

All DAO interfaces must extend the GenericDAO, which is specifying the type of the handled object and the type of the ID, that is usually a Long. All DAO implementations should extend GenericDAOImpl to avoid re-implementing the interface and to get access to additional helper methods.

All DAOs should provide list methods by default for public use services, with all parameters included that may be required to search a specific object. The list method must return a Page containing the content as a list of items and  PagingInfo. It must support a Paging parameter (last parameter in list), and should support a Sorting parameter (normally before the Paging-parameter). List method internally should do count check and list items only if the count > 0. Sorting requires additional setup in the objects, to define the mapping for the sortable columns. Avoid the use of a count method, use it only when there is specific need.

The DAO implementation should have a @Component("dao-name") annotation, so the DAO can be used both with XML and annotation-based configurations in products.

Controller

General

  • All controllers must extend BaseController. All Controller must be annotated with @RequestMapping to define the base path, which will be defined in the WalletApi_1_0_0 interface, e. g. WalletApi_1_0_0.BASE_PATH_WALLET.
  • All controllers need to define their origin, which is ApiRequestInfo.ExternalAPI for public controllers, and ApiRequestInfo.InternalAPI for admin only(!) controllers.
  • All controllers must log constructor calls to have a better overview in the log files.
  • In general, for a http-exposed service there is a MyFeatureApiController and a MyFeatureApiAdminController to separate mapping/origin, as mentioned above.
  • All controllers should have a @RestController annotation, so the controller can be used both with XML and annotation-based configurations in products

HttpInvoker

The HttpInvoker-export-bean must be declared like this, so that it properly validates and sets the SecurityContext:

<bean id="wallet.impl.HttpInvokerRemoteInvocationExecutor" class="com.trimplement.wallet.server.common.httpinvoker.AuthenticatingRemoteInvocationExecutor" />
<bean id="wallet.impl.BaseHttpInvoker" class="org.springframework.remoting.httpinvoker.HttpInvokerServiceExporter" abstract="true">
    <property name="remoteInvocationExecutor" ref="wallet.impl.HttpInvokerRemoteInvocationExecutor" />
</bean>    
    
<bean id="wallet.impl.AuthenticationService.httpInvoker" parent="wallet.impl.BaseHttpInvoker">
    <property name="service" ref="wallet.impl.AuthenticationService" />
    <property name="serviceInterface" value="com.trimplement.wallet.server.wallet.api.authentication.AuthenticationService" />
</bean>

Logging

  • Entry/exit logging should be done with the @Log annotation.
  • Use SLF4J for logging.
  • If it is required to do custom logging inside methods use Lombok @Slf4j annotation on the class level, This provides a generated logger object to the class. Do not create a logger object manually.
import lombok.extern.slf4j.Slf4j;
@Slf4j
public class Example {
    public void doSomething() {
        logger.info("do something");   
    }
}
  • Use placeholders ‘{}’  to pass arguments. SLF4J will create the log message string only if the corresponding level is enabled for the logger class. No further checking using isXXXEnabled() is needed for performance reasons.
logger.info("Hello firstName:{}, lastName:{}", firstName, lastName)
  • Do not use String.format() or concatenation to prepare the log message, this way string messages will be prepared in advance irrespective of whether the log level is enabled or not.
  • Use the exception as the last argument in order to log the exception trace, don’t use placeholder ‘{}’ for exception if you want the exception trace.
logger.error("Unable to parse data:{}", data, exception);
  • Use the placeholder ‘{}’ for exception, only if the exception message must be logged and no exception trace.
logger.error("Unable to parse data:{}, exception:{}", data, exception.getMessage());
  • SLF4J supports fatal() logging through Marker. So in order to log fatal level messages, use the following approach.
logger.fatal("fatal error");
//should be changed to following.
logger.error(CommonConstants.FATAL, "fatal error");
// CommonConstants FATAL is declared as follows.
private static final Marker FATAL = MarkerFactory.getMarker("FATAL");
  • Only log stack-traces for unexpected exceptions (e.g. runtime exception, illegal configuration, etc), otherwise the log will be spammed with stack-traces for expected situations, more important stack-traces will be very hard to spot, the log will be very hard to read, and performance will suffer due to the enormous amount of logging