Skip to content
This repository has been archived by the owner on Mar 20, 2024. It is now read-only.

CodeGuidelines

Jump to bottom
Michael Ekstrand edited this page Mar 25, 2014 · 14 revisions

LensKit's code follows pretty standard Java practice and style, with some particular details we additionally care about. This document is also expected to grow and expand as we figure out how to describe more patterns we are using.

Not all code in LensKit currently conforms to this style; conformity is an ongoing process. When we're working on a piece of code and find violations, we often go ahead and correct them, so long as it will not make the resulting diff too messy (e.g. add braces to control structures while reviewing/editing code).

In addition to the things documented here, we strongly recommend reading Effective Java. We try to follow a great deal of its advise in LensKit.

Compatibility

LensKit is compatible with Java 6. All LensKit code should run on Java 6 SE, using only dependencies available via Maven Central.

We optimize for Java 7 — it is acceptable to write code that is faster in 7, or requires 7 to be fully tuned, so long as it still works correctly in 6 (e.g. expecting Collections.sort to be O(n) for already-sorted lists, which is true of the TimSort algorithm used by Java 7). Currently, we do not optimize for Java 8 (at least not at the expense of Java 7).

Our continuous integration builds on Travis build on Java 6, Java 7, and Java 8, failing if the build and integration tests fail on any of those platforms.

Naming

  • Classes should be named in UpperCamelCase
  • Methods and fields should be named in lowerCamelCase
  • Constants (static final fields) should be in UPPERCASE_WITH_UNDERSCORES.
  • Packages should be lowercase; we don't like multi-word components of a package name.

Formatting

  • Use 4-space indents, not tabs. There are some tabs in the code base, but these are deprecated.

  • Put opening brace on the end of the line:

    while (someCondition) {
    doStuff();
}

    This applies for all structures — classes, methods, control structures. If there is a really long method header or class declaration, placing the opening brace on the following line is acceptable.

  • Group closing brace with else:

    if (someCondition) {
    foo();
} else {
    bar();
}

  • All control flow structures (conditionals and loops) must use braces or be entirely (condition and statement) on one line. That is, do this:

    if (someCondition) {
    foo();
}

    not this:

    if (someCondition)
    foo();

    but this is acceptable:

    if (someCondition) return;

    The reason for this guideline is that it is easier to review patches that modify conditionals if all conditional statements have braces.

  • Some spacing rules

    • Put spaces between if, while, for and the parenthesis (as in previous rule).
    • Do not put spaces between method name and parenthesis. Do this(), not this ().
    • Don't put space immediately in parentheses. Do this(foo), not this( foo ). Same applies to the parentheses on control structures.
    • Other spacing in expressions is based on “what's most readable”. We generally put spaces after commas, but not always.
    • Put a single space before the opening brace on a line.

  • Wrap lines as appropriate. We prefer lines under 100 characters; a little is OK when most readable, but 150 is too long. @elehack has his editor set to have a margin line at 100 characters.

  • Comment when appropriate but no more. Comments should explain why we need to do something, particularly if it is not obvious, and should explain the strategy used in methods. We prefer clear code with meaningful names to comments that repeat the code.

  • Don't use this unless it's needed to disambiguate.

Object Design

  • Whenever possible, objects should be immutable. Immutability keeps things easy.
  • Make equals() relationships, when defined, well-defined, even if that detracts from their practicality. If you can't make a usable, well-defined equals() method, and need one, the design may need some rethinking. See How to Write an Equality Method in Java for information on some of the nuances of equality methods. They're really hard to get right.
  • Prefer composition over inheritance, and the Strategy pattern over protected method overrides. This is not to say we never use inheritance, but if behavior needs to be configurable, it should be split into a separate strategy interface/component if practical.

In general, we try follow a lot of the advice in Effective Java, and are working on improving the LensKit APIs to better conform to these principles.

Unit Tests

  • Use JUnit 4
  • Prefer assertThat over assertEquals in general
  • Use assertThat(x, equalTo(y)) for integers
  • Use assertThat(x, closeTo(y)) for floating point
  • Use assertThat(x, notANumber()) for NaN comparisons (notANumber() is a GroupLens matcher)

Eclipse

  • The XML file attached below gives some settings that will make Eclipse work close to these guidelines. To use these settings: go to Windows | Preferences | Java | Code Style | Formatter and import the XML file (after saving it to your disk).
  • It is good to turn off tabs in Eclipse, since they are unpredictable on different systems. Go to Windows | Preferences | General | Editors | Text Editors and choose "Insert spaces for tabs"

Committing Code

  • Use your name and e-mail address as the author. Git will prompt you to set this up automatically.

  • Make sure each commit compiles. This isn't a hard requirement, as we're using distributed version control, but it is helpful. git bisect benefits greatly from compiling commits.

Commit Messages

Tim Pope has some good advice about commit messages. In LensKit, we don't care as much about 72-char lines, but we do like the short first line for a subject.

A good commit message should therefore look like this:

Short description (like an e-mail subject)

More detailed description.
- with bulleted changes
- but only if it helps explain the project

If the commit pertains to a ticket, it's good to include the ticket number (as #N) somewhere to link the commit to the ticket. That will make GitHub automatically associate the commit with the relevant ticket.

Clone this wiki locally