CodeGuidelines

LensKit Code Guidelines

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).

Compatibility

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

We sometimes optimize for 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).

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`:
{{{
#!java
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:
{{{
#!java
if (someCondition) {
foo();
}
}}}
not this:
{{{
#!java
if (someCondition)
foo();
}}}
but this is acceptable:
{{{
#!java
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 80 characters; longer is OK when most readable, but 120 is too long.
* 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

* When it makes sense, 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 [[http://www.artima.com/lejava/articles/equality.html|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 [[http://en.wikipedia.org/wiki/Strategy_pattern|Strategy pattern]] over protected method overrides.  This is not to say we never do inheritance, but if behavior needs to be configurable, it should be split into a separate strategy interface/component if practical.

## Unit Tests

* Use JUnit 4
* Prefer assertThat 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. This can be done in `~/.hgrc`:
  {{{
  [ui]
  username = Michael Ekstrand <ekstrand@cs.umn.edu>
  }}}

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

### Commit Messages

Tim Pope has some [http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html good advice] about commit messages; it's for Git, but applies to Mercurial as well. 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 `refs #N` somewhere to link the commit to the ticket. That will make Trac automatically associate the changeset with the relevnant ticket.