-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
CodeStyle
We're currently using the code style as specified in settings.jar.
In Android Studio go to File > Import settings… and select the file. It will create a new code style profile called "K-9 Mail". None of your global settings will be overwritten!
This webpage formalises the intended code style for the project. Increasingly these are being enforced by build-time linting tools like CheckStyle.
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.
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.*Class Names are expected to be upper camel-case starting (e.g. BaseAccount).
We prefer lowercasing initials (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.
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.
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 but it is not good practice.
Constants are expected to be upper snake case (e.g. ACCOUNT_TYPE) but otherwise follow the rules for variables.
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 (property) {
case ‘1’:
break;
case ‘2’:
default:
break;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.
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.
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.
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)
We would like to move towards a situation where these tools are enabled and fail the build if they raise 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 these tools to ensure you aren’t adding more stuff to fix before these can be enabled.
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.
At the time of this writing (2017-03-23) the code style is not enforced and much of the code base doesn't conform to it, e.g. in many places fields are still prefixed with "m".
If your changes touch a lot of files please don't reformat all of the affected files to use the new code style as this will make it much harder to merge existing branches. You should, however, try to use the code style for new code.
If you decide to fix the code style of a file/method, please do so in a separate commit that doesn't contain any functional changes.
Note: It would be great if you could extract the code style rules from the settings file and put them here in human readable form.
- 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.
- Do not use
K9.appin new code!
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.
We're trying to make sure all source and resource files consistently use LFs as line delimiters to avoid merge conflicts.
On Windows, the easiest way to handle this is to have git do it for you. By executing
$ git config --global core.autocrlf true
at any command prompt. This will cause git to checkout files using Windows-style line endings (CRLF), but all checkins will convert line endings to LF.
On Mac and Linux, you can use
$ git config --global core.autocrlf input
to make sure you only commit LF line endings.