The following guidelines are used for Orekit development.
Orekit is a low level library. It may be used in very different contexts which cannot be foreseen, from quick studies up to critical operations. The main driving goals are the following ones:
The first goal, validation, implies tests must be as extensive as possible. They should include realistic operational cases but also contingency cases. The jacoco tool must be used to monitor test coverage. A very high level of coverage is desired. We do not set up mandatory objective figures, but only guidelines here. However,a 60% line coverage would clearly not be acceptable at all and 80% would be considered deceptive.
The second goal, robustness, has some specific implications for a low level component like Orekit. In some sense, it can be considered an extension of the previous goal as it can also be improved by testing. It can also be improved by automatic checking tools that analyze either source code or binary code. The findbugs tool is already configured for automatic checks of the library using a maven plugin.
This is however not sufficient. A library is intended to be used by applications unknown to the library development team. The library development should be as flexible as possible to be merged into an environment with specific constraints. For example, perhaps an application should run 24/7 during months or years, so caching all results to improve efficiency may prove disastrous in such a use case, or it should be embedded in a server application, so printing to standard output should never be done. Experience has shown that developing libraries is more difficult than developing high level applications where most of the constraints are known beforehand.
The third goal, maintainability, implies code must be readable, clear and well documented. Part of this goal is enforced by the stylistic rules explained in the next section, but this is only for the automatic and simple checks. It is important to keep a clean and extensible design. Achieving simplicity is really hard, so this goal should not be taken too lightly. Good designs are a matter of balance between two few objects that do too many things internally an ignore each other and too many objects that do nothing alone and always need a bunch of other objects to work. Always think in terms of balance, and check what happens if you remove something from the design. Quite often, removing something improves the design and should be done.
The fourth goal, efficiency, should be handled with care to not conflict with the second and third goals (robustness and maintainability). Efficiency is necessary but trying too much too achieve it can lead to overly complex unmaintainable code, to too specific fragile code, and unfortunately too often without any gain after all because of premature optimization and unfounded second-guess.
One surprising trick, that at first sight might seem strange and inappropriate has been used in many part for Orekit and should be considered a core guideline. It is the use of immutable objects. This trick improves efficiency because many costly copying operation are avoided, even unneeded one added for defensive programming. It improves maintainability because both the classes themselves and the classes that use them are much simpler. It also improves robustness because many (really many ...) difficult to catch bugs are caused by mutable objects that are changed in some deeply buried code and have an impact on user code that forgot to perform a defensive copy. Orbits, dates, vectors, and rotations are all immutable objects.
Source Control Management¶
This implies that development occurs on a develop branch only. Developers create feature branches, and merge them on the develop branch when ready. The develop branch should always be functional so people wanting to be on the bleeding edge can use it (for example to create nightly builds or to prepare their application for upcoming features before the official release).
In order to improve traceability, when a feature branch is merged into develop, it should use
git merge ---no-ff
the --no-ff flag prevents fast-forward so the history makes it clear there was a feature branch at this point.
Feature branches are temporary, they are deleted when they are not useful anymore. Feature branches may be present only in one developer workspace and never appear in the main repository, however, they may also be pushed on the main repository if the developer wants to share work or needs community feedback.
When a release is desired, a dedicated branch should be created, with a name following the pattern release-x.y. These branches are created from the develop branch. When the release is ready, the branch is merged both into the master branch and into the develop branch. Once a release branch has been set up, it will remain. This allows users relying on this specific version to be able to retrieve the fixes published afterward (i.e. x.y.1, x.y.2...).
After a release has been published and pushed to master, it may be necessary to publish an urgent bugfix if a serious problem in the released version is found. Short-lived bugfix branches that are created directly from master are devoted to this. These bugfix branches are merged back into master and develop.
The master branch always refer to the latest stable release performed. No direct work is done on this branch. It is updated only by merging either release branches or bugfix branches branches to it.
For reading ease and consistency, the existing code style should be preserved for all new developments. The rules are common ones, inherited mainly from the Sun Code Conventions for the Java Programming Language guide style and from the default checkstyle tool configuration. A few of these rules are displayed below. The complete definition is given by the checkstyle configuration file in the project root directory.
all source files start with the Apache license header,
no tabs, 4 spaces indentation, no indentation for case statements,
operators wrapping rules
lines are wrapped after operators (unlike Sun),
operators are surrounded by spaces, method parameters open parenthesis is not preceded by space, lines do not end with white space,
curly brace rules
open curly brace are at end of line, with the matching closing curly brace aligned with the start of the corresponding keyword (if, for, while, case or do),
characters encoding is UTF8, the git property core.autocrlf should be set to input on Linux development machines and to true on Windows development machines (to ensure proper conversion on all operating systems),
classes names begin with upper case, instance methods and fields names begin with lower case, class fields are all upper case with words separated by underscores,
class variables come first, followed by instance variables, followed by constructors, and followed by methods, public modifiers come first, followed by protected modifiers followed by private modifiers,
all elements have complete javadoc, even private fields and methods (there are some rare exceptions, in case of code translated from the fortran language and models with huge parameters sets),
switch/case construct have a default argument, even when all possible cases are already handled, as many classes as possible are immutable,
star imports are forbidden, parameters and local variables are final wherever possible.
seek for a line test coverage of at least 80% (more is better)
fix all errors and warnings found by findbugs
no runtime assumptions (robustness)
do not make assumptions on the runtime environment of applications using Orekit (they may be embedded with no console, no possible user interaction, no network, no writable file system, no stoppable main program, have memory constraints, time constraints, be run in different linguistic contexts ...)
follow Occam's razor principle or its declination in computer science: KISS (Keep It Simple, Stupid)
balanced design (efficiency)
seek efficiency, but do not overstep robustness and maintainability
immutable objects (robustness, maintainability)
use immutable objects as much as possible
fix all errors and warnings found by checkstyle