The code should follow general conventions for Java (see Oracle Naming Conventions, Google Java Style, etc.).We consider this as common sense and provide configurations for SonarQube and related tools such as Checkstyle instead of repeating this here.
Besides general Java naming conventions, we follow the additional rules listed here explicitly:
-
Always use short but speaking names (for types, methods, fields, parameters, variables, constants, etc.).
-
For package segments and type names prefer singular forms (
CustomerEntityinstead ofCustomersEntity). Only use plural forms when there is no singular or it is really semantically required (e.g. for a container that contains multiple of such objects). -
Avoid having duplicate type names. The name of a class, interface, enum or annoation should be unique within your project unless this is intentionally desired in a special and reasonable situation.
-
Avoid artificial naming constructs such as prefixes (
I*) or suffixes (*IF) for interfaces. -
Use CamlCase even for abbreviations (
XmlUtilinstead ofXMLUtil) -
Names of Generics should be easy to understand. Where suitable follow the common rule
E=Element,T=Type,K=Key,V=Valuebut feel free to use longer names for more specific cases such asID,DTOorENTITY. The capitalized naming helps to distinguish a generic type from a regular class.
Java Packages are the most important element to structure your code. We use a strict packaging convention to map technical layers and business components (slices) to the code (See technical architecture for further details). By using the same names in documentation and code we create a strong link that gives orientation and makes it easy to find from business requirements, specifications or story tickets into the code and back.
For an devon4j based application we use the following Java-Package schema:
«rootpackage».«application».«component».«layer».«scope»[.«detail»]*E.g. in our example application we find the Spring Data repositories for the ordermanagement component in the package com.devonfw.application.mtsj.ordermanagement.dataaccess.api.repo
| Segment | Description | Example |
|---|---|---|
«rootpackage» |
Is the basic Java Package name-space of the organization or IT project owning the code following common Java Package conventions. Consists of multiple segments corresponding to the Internet domain of the organization. |
|
«application» |
The name of the application build in this project. |
|
«component» |
The (business) component the code belongs to. It is defined by the business architecture and uses terms from the business domain. Use the implicit component |
|
«layer» |
The name of the technical layer (See technical architecture) which is one of the predefined layers ( |
|
«scope» |
The scope which is one of |
|
«detail» |
Here you are free to further divide your code into sub-components and other concerns according to the size of your component part. |
|
Please note that for library modules where we use com.devonfw.module as «basepackage» and the name of the module as «component». E.g. the API of our beanmapping module can be found in the package com.devonfw.module.beanmapping.common.api.
We combine the above naming and packaging conventions to map the entire architecture to the code.
This also allows tools such as CobiGen or sonar-devon4j-plugin to "understand" the code.
Also this helps developers going from one devon4j project to the next to quickly understand the code-base.
If every developer knows where to find what, the project gets more efficient.
A long time ago maven standardized the project structure with src/main/java, etc. and turned chaos into structure.
With devonfw we experienced the same for the codebase (what is inside src/main/java).
«rootpackage».«application» ├──.«component» | ├──.common | | ├──.api[.«detail»] | | | ├──.datatype | | | | └──.«Datatype» | | | └──.«BusinessObject» | | └──.impl[.«detail»] | | ├──.«Datatype»JsonSerializer | | └──.«Datatype»JsonDeserializer | ├──.dataaccess | | ├──.api[.«detail»] | | | ├──.repo | | | | └──.«BusinessObject»Repository | | | ├──.dao (alternative to repo) | | | | └──.«BusinessObject»Dao (alternative to Repository) | | | └──.«BusinessObject»Entity | | └──.impl[.«detail»] | | ├──.dao (alternative to repo) | | | └──.«BusinessObject»DaoImpl (alternative to Repository) | | └──.«Datatype»AttributeConverter | ├──.logic | | ├──.api | | | ├──.[«detail».]to | | | | ├──.«MyCustom»«To | | | | ├──.«DataStructure»Embeddable | | | | ├──.«BusinessObject»Eto | | | | └──.«BusinessObject»«Subset»Cto | | | ├──.[«detail».]usecase | | | | ├──.UcFind«BusinessObject» | | | | ├──.UcManage«BusinessObject» | | | | └──.Uc«Operation»«BusinessObject» | | | └──.«Component» | | ├──.base | | | └──.[«detail».]usecase | | | └──.Abstract«BusinessObject»Uc | | └──.impl | | ├──.[«detail».]usecase | | | ├──.UcFind«BusinessObject»Impl | | | ├──.UcManage«BusinessObject»Impl | | | └──.Uc«Operation»«BusinessObject»Impl | | └──.«Component»Impl | └──.service | ├──.api[.«detail»] | | ├──.rest | | | └──.«Component»RestService | | └──.ws | | └──.«Component»WebService | └──.impl[.«detail»] | ├──.jms | | └──.«BusinessObject»JmsListener | ├──.rest | | └──.«Component»RestServiceImpl | └──.ws | └──.«Component»WebServiceImpl ├──.general │ ├──.common │ | ├──.api | | | ├──.to | | | | ├──.AbstractSearchCriteriaTo | | | └──.ApplicationEntity │ | ├──.base | | | └──.AbstractBeanMapperSupport │ | └──.impl │ | ├──.config │ | | └──.ApplicationObjectMapperFactory │ | └──.security │ | └──.ApplicationWebSecurityConfig │ ├──.dataaccess │ | └──.api | | └──.ApplicationPersistenceEntity │ ├──.logic │ | └──.base | | ├──.AbstractComponentFacade | | ├──.AbstractLogic | | └──.AbstractUc | └──.service | └──... └──.SpringBootApp
Code spots that need some rework can be marked with the following tasks tags. These are already properly pre-configured in your development environment for auto completion and to view tasks you are responsible for. It is important to keep the number of code tasks low. Therefore every member of the team should be responsible for the overall code quality. So if you change a piece of code and hit a code task that you can resolve in a reliable way do this as part of your change and remove the according tag.
Used to mark a piece of code that is not yet complete (typically because it can not be completed due to a dependency on something that is not ready).
// TODO «author» «description»A TODO tag is added by the author of the code who is also responsible for completing this task.
// FIXME «author» «description»A FIXME tag is added by the author of the code or someone who found a bug he can not fix right now. The «author» who added the FIXME is also responsible for completing this task. This is very similar to a TODO but with a higher priority. FIXME tags indicate problems that should be resolved before a release is completed while TODO tags might have to stay for a longer time.
// REVIEW «responsible» («reviewer») «description»A REVIEW tag is added by a reviewer during a code review. Here the original author of the code is responsible to resolve the REVIEW tag and the reviewer is assigning this task to him. This is important for feedback and learning and has to be aligned with a review "process" where people talk to each other and get into discussion. In smaller or local teams a peer-review is preferable but this does not scale for large or even distributed teams.
As a general goal the code should be easy to read and understand. Besides clear naming the documentation is important. We follow these rules:
-
APIs (especially component interfaces) are properly documented with JavaDoc.
-
JavaDoc shall provide actual value - we do not write JavaDoc to satisfy tools such as checkstyle but to express information not already available in the signature.
-
We make use of
{@link}tags in JavaDoc to make it more expressive. -
JavaDoc of APIs describes how to use the type or method and not how the implementation internally works.
-
To document implementation details, we use code comments (e.g.
// we have to flush explicitly to ensure version is up-to-date). This is only needed for complex logic.
This section gives you best practices to write better code and avoid pitfalls and mistakes.
Avoid using byte[] for BLOBs as this will load them entirely into your memory. This will cause performance issues or out of memory errors. Instead use streams when dealing with BLOBs. For further details see BLOB support.
Resources such as streams (InputStream, OutputStream, Reader, Writer) or transactions need to be handled properly. Therefore it is important to follow these rules:
-
Each resource has to be closed properly, otherwise you will get out of file handles, TX sessions, memory leaks or the like
-
Where possible avoid to deal with such resources manually. That is why we are recommending
@Transactionalfor transactions in devonfw (see Transaction Handling). -
In case you have to deal with resources manually (e.g. binary streams) ensure to close them properly. See the example below for details.
Closing streams and other such resources is error prone. Have a look at the following example:
try {
InputStream in = new FileInputStream(file);
readData(in);
in.close();
} catch (IOException e) {
throw new RuntimeIoException(e, IoMode.READ);
}The code above is wrong as in case of an IOException the InputStream is not properly closed. In a server application such mistakes can cause severe errors that typically will only occur in production. As such resources implement the AutoCloseable interface you can use the try-with-resource syntax to write correct code. The following code shows a correct version of the example:
try (InputStream in = new FileInputStream(file)) {
readData(in);
} catch (IOException e) {
throw new RuntimeIoException(e, IoMode.READ);
}With Java8 you have cool new feautres like lambdas and monads like (Stream, CompletableFuture, Optional, etc.).
However, these new features can also be misused or lead to code that is hard to read or debug. To avoid pain, we give you the following best practices:
-
Learn how to use the new features properly before using. Often developers are keen on using cool new features. When you do your first experiments in your project code you will cause deep pain and might be ashamed afterwards. Please study the features properly. Even Java8 experts still write for loops to iterate over collections, so only use these features where it really makes sense.
-
Streams shall only be used in fluent API calls as a Stream can not be forked or reused.
-
Each stream has to have exactly one terminal operation.
-
Do not write multiple statements into lambda code:
collection.stream().map(x -> { Foo foo = doSomething(x); ... return foo; }).collect(Collectors.toList());
This style makes the code hard to read and debug. Never do that! Instead extract the lambda body to a private method with a meaningful name:
collection.stream().map(this::convertToFoo).collect(Collectors.toList());
-
Do not use
parallelStream()in general code (that will run on server side) unless you know exactly what you are doing and what is going on under the hood. Some developers might think that using parallel streams is a good idea as it will make the code faster. However, if you want to do performance optimizations talk to your technical lead (architect). Many features such as security and transactions will rely on contextual information that is associated with the current thread. Hence, using parallel streams will most probably cause serious bugs. Only use them for standalone (CLI) applications or for code that is just processing large amounts of data. -
Do not perform operations on a sub-stream inside a lambda:
set.stream().flatMap(x -> x.getChildren().stream().filter(this::isSpecial)).collect(Collectors.toList()); // bad set.stream().flatMap(x -> x.getChildren().stream()).filter(this::isSpecial).collect(Collectors.toList()); // fine
-
Only use
collectat the end of the stream:set.stream().collect(Collectors.toList()).forEach(...) // bad set.stream().peek(...).collect(Collectors.toList()) // fine
-
Lambda parameters with Types inference
(a,b,c) -> a.toString() + Float.toString(b) + Arrays.toString(c) // fine (String a, Float b, Byte[] c) -> a.toString() + Float.toString(b) + Arrays.toString(c) //bad Collections.sort(personList, (p1, p2) -> p1.getSurName().compareTo(p2.getSurName())); //fine Collections.sort(personList, (Person p1, Person p2) -> p1.getSurName().compareTo(p2.getSurName())); //bad
-
Avoid Return Braces and Statement
(a) -> a.toString(); // fine (a) -> { return a.toString(); } //bad
-
Avoid Parentheses with Single Parameter
a -> a.toString(); // fine (a) -> a.toString(); //bad
-
Avoid if/else inside foreach method. Use Filter method & comprehension
Bad static public Iterator<String> TwitterHandles(Iterator<Author> authors, string company) { final List result = new ArrayList<String> (); foreach (Author a : authors) { if (a.Company.equlas(company)) { String handle = a.TwitterHandle; if (handle != null) result.Add(handle); } } return result; }
Fine public List<String> twitterHandles(List<Author> authors, String company) { return authors.stream() .filter(a -> null != a && a.getCompany().equals(company)) .map(a -> a.getTwitterHandle()) .collect(toList()); }
With Optional you can wrap values to avoid a NullPointerException (NPE). However, it is not a good code-style to use Optional for every parameter or result to express that it may be null. For such case use @Nullable or even better instead annotate @NotNull where null is not acceptable.
However, Optional can be used to prevent NPEs in fluent calls (due to the lack of the elvis operator):
Long id;
id = fooCto.getBar().getBar().getId(); // may cause NPE
id = Optional.ofNullable(fooCto).map(FooCto::getBar).map(BarCto::getBar).map(BarEto::getId).orElse(null); // null-safeEncoding (esp. Unicode with combining characters and surrogates) is a complex topic. Please study this topic if you have to deal with encodings and processing of special characters. For the basics follow these recommendations:
-
When you have explicitly decide for an encoding always prefer Unicode (UTF-8 or better). This especially impacts your databases and has to be defined upfront as it typically can not be changed (easily) afterwards.
-
Do not cast from
bytetochar(Unicode characters can be composed of multiple bytes, such cast may only work for ASCII characters) -
Never convert the case of a String using the default locale (esp. when writing generic code like in devonfw). E.g. if you do
"HI".toLowerCase()and your system locale is Turkish, then the output will be "hı" instead of "hi" what can lead to wrong assumptions and serious problems. If you want to do a "universal" case conversion always use explicitly an according western locale (e.g.toLowerCase(Locale.US)). Consider using a library (https://github.com/m-m-m/util/blob/master/core/src/main/java/net/sf/mmm/util/lang/api/BasicHelper.java) or create your own little static utility for that in your project. -
Write your code independent from the default encoding (system property
file.encoding) - this will most likely differ in JUnit from production environment-
Always provide an encoding when you create a
Stringfrombyte[]:new String(bytes, encoding) -
Always provide an encoding when you create a
ReaderorWriter:new InputStreamReader(inStream, encoding)
-
Avoid unnecessary strong bindings:
-
Do not bind your code to implementations such as
VectororArrayListinstead ofList -
In APIs for input (=parameters) always consider to make little assumptions:
-
prefer
CollectionoverListorSetwhere the difference does not matter (e.g. only useSetwhen you require uniqueness or highly efficientcontains) -
consider prefering
Collection<? extends Foo>overCollection<Foo>whenFoois an interface or super-class
-