CodeStyle

K-9 Code Style

Try to follow the existing code style.

Kotlin

The project is mostly following Kotlin's coding conventions.

We're using spotless with ktlint to enforce formatting rules. You can use the Gradle task spotlessCheck to check the code and the task spotlessApply to automatically fix violations.

Enum or Sealed Class/Interface

When you need to decide which one to pick, follow this rules:

• Use enum when you have a simple set of immutable constants.
• Use sealed interface/class when you need more flexibility, or when you need to attach state or behavior to the constants.

Java

Try to avoid writing new Java code as much as possible.

Package Names

com.fsck.k9 is the top level package.

Sub-package names should be single ASCII English words or rarely two words, all lowercase, with no hyphen or other separator.

Imports

The order and style of imports should be:

import java.util....

import a.b.c

import b.c.d
import b.d.e

import com.fsck.k9

import static org.mockito.Mockito.*

Classes

Class Names are expected to be upper camel-case starting (e.g. BaseAccount).

We prefer treating acronyms as normal words in regard to capitalization (hence ImapAccount not IMAPAccount).

In general, despite the use of packages, classes should be unique across the project where possible.

Prefer extracting classes to separate files where reasonable. Longer classes are less easy to maintain and harder to test individual features.

Methods

Method names should be lower camel-case (e.g. performQuery).

Most method names start with a verb. Well named methods are preferable to short names.

Methods should be separated from each other by a single new line.

Variable Names

Variables should be lower camel-case. (e.g. accountName).

They do not need to indicate the variable type (e.g. not accountNameStr or similar).

They also should not be prefixed by m (e.g. not mAccountName). The codebase contains many examples of this for legacy reasons, but it is not good practice.

Constants

Constants are expected to be upper snake case (e.g. ACCOUNT_TYPE) but otherwise follow the rules for variables.

If / Else

The expected format of an if-else block is as follows:

if (conditionIsTrue) {
    executeCommand();
} else {
    handleAlternative();
}

Note the space between the if and the open-paren, the close-paren and the bracket and the else’s position on the same line.

Switch / Case

switch (property) {
    case "1": {
	break;
    }
    case "2":
    // fallthrough should be used rarely, and noted by a comment:

    // fallthrough
    default: {
	break;
    }
}

Comments

Single line comments should be used sparingly. Prefer splitting up methods into smaller helper methods than writing comments. Having to explain what ‘this bit of the method does’ is generally a sign the method is too long.

Multi-line comments should be used even more rarely and might indicate a refactor would be useful.

Good use cases for comments are for noting work-arounds for bugs in Android.

TODO / FIXME / etc

For legacy reasons, there exists a number of TODOs in the code.

In general these are probably tracked by issues.

They almost never represent ‘easy fixes’ - many of them would require fundamental reworks.

New code should not add TODOs and will be rejected if it does. If a change leaves scope for improvement a GitHub issue should track that instead.

Javadoc

We don’t publish the Javadoc currently. As such almost nothing needs Javadoc. Certainly classes only consumed by K-9 don’t need Javadoc.

At some point we may write proper Javadoc for Providers and some of the K-9 Mail Library. But this will be properly designed and planned before-hand.

Unless the GitHub issue specifically mentions writing Javadoc, don’t write it. And delete any stuff auto-generated by IDEs.

The primary issue with it is:

• It typically doesn’t tell you much more than the method signature
• It is easy for the documentation to actively mislead, especially following fixes and refactoring
• There’s no clear target audience
• It makes class files even longer.

XML

The resource structure is well defined (with the exception of the long strings.xml file).

Resource names are lower snake-case (e.g. some_text)

Lint

We would like to move towards a situation where this tool is enabled and fails the build if it raises errors. However we are not there yet.

New code should not add to the existing counts substantially. If you write large amounts of functionality consider at least running Lint to ensure you aren’t adding more stuff to fix before this can be enabled.

Automated Testing

Because of the nature of Android it is impractical to test on every device. In addition, email servers have quirks. For this reason we rely on a growing suite of automated tests.

Passing the automated tests is mandatory for code to be merged.

For new functionality we would like to see automated tests covering the new behaviour (i.e. if an option is added, tested both with and without it).

Much of the code is however still not subject to automated testing for a variety of reasons. Efforts to improve this situation are appreciated.

Other notes on general code quality

• Use meaningful names for fields, variables, methods and classes.
• Try to keep methods small.
• Try to keep classes small.
• Avoid comments. If you need comments to explain something, chances are your code needs more meaningful names. If you're working around quirks/bugs in Android or a library, please do add a comment explaining why your work-around is necessary.

Over the years many different people have worked on K-9 Mail. In many cases K-9 Mail's code base is not a good example on how to write clean code. You're welcome to improve this situation with pull requests!

Please don't be afraid to submit pull requests even if you believe you can't meet the code quality standards we aspire to. Github's comment system for pull requests is a good tool that allows us to work together on improving the pull request.