Coding guidelines - Pyosch/powertac-server GitHub Wiki

Power TAC coding style guidelines

This style guide is a set of suggestions for program layout and formatting. Uniform style makes it easier for us to read and understand each other's code.

Here are the most important highlights:

  1. The basic indentation is two spaces. You should be able to get your editor to do that for you, but please don't set the tab length to two. Leave it at 8 so the code is correctly formatted in everyone's IDE and on the printer. If you are using STS, you should configure the style with powertac-server/powertac-style.xml - you can import it under Window->Preferences->Java->CodeStyle->Formatter. In IntelliJ IDEA, configure your Code Style > General settings as follows: (check) Use the same settings for all file types. Tab size: 2, Indent: 2, Continuation indent: 4, Label indent: 0. Leave all other boxes unchecked.
  2. Package names are all lowercase, and preferably short - six characters or less.
  3. Variable and method names are camel-case, beginning with a lowercase letter. Don't use uppercase acronyms like RFQ in names - instead, use rfq at the beginning of a name, or Rfq in the middle.
  4. Short variable names like i or k should be used only for loop indices or other very limited scopes, and then only for scopes that are short enough to read without scrolling.
  5. Class and interface names are capitalized camel-case.
  6. Constant names are uppercase, with words separated by underscores.
  7. There are spaces after keywords and around binary operators.
  8. We don't use magic numbers.
  9. Every class, and every public method is introduced with a javadoc comment. Note that public implementations of methods declared in an interface will inherit the javadoc from the interface declaration, so put your javadocs in the interface. If you are overriding a method in a subclass, include a javadoc for the override method.
  10. Except in unusual circumstances, methods should be less than a page in length.
  11. All data fields are private or protected, except that inner classes that act as simple data holders can have public fields.

File layout

Organize the material in each file as follows:

  • Header, including a one-line description, id tag, and copyright statement
  • package statement
  • import statements
  • A javadoc comment explaining the purpose of this file
  • The class declaration itself

Classes

In every class, it usually makes sense to first list all public features, then all protected and private features. Exceptions include cases where there is both a public and a private way of doing the same thing; then you would put the private version directly after the public version, and make the commentary clear about the fact that they go together.

List features in the following order:

  • public static final constants (see the state constants in edu.umn.cs.tac.repository.Repository for an example)
  • member variables
  • instance methods
  • static methods
  • static (class) variables (see below)
  • inner classes

Leave a blank line after every function, and use whitespace appropriately elsewhere. Avoid multiple blank lines. Instead, use section separators to split up the presentation into sections that hang together. If there are a lot of them, perhaps you should create multiple classes.

Visibility

In most cases, member and static variables should be private. The only reason for making them protected is to make them available to subclasses. Protected member variables should be preceded by a javadoc comment to help the designer of the subclass understand what it's for and why a subclass might want to access it directly. Even better, if you are tempted to use protected methods and variables to make elements visible to subclasses, then the superclass should include a detailed "specialization protocol" describing the rules for creating subclasses correctly.

To make a member variable visible outside the class, write a getter and possibly a setter on it.

The only legitimate use of default visibility (absence of public, private, or protected) is to make elements of an inner class visible to the enclosing class.

Methods and constants can be public, protected, or private, as appropriate.

Supply a default constructor for every class, and begin every constructor with the super() call.

Avoid static (class) variables whenever possible. One obvious exception to this is the instance reference in a Singleton.

Methods

Every public or protected method (except perhaps for main) should be introduced with a comment in javadoc format. See the javadoc writeup and try to follow the writing style guidelines given there. Understand what javadoc does, and think about how the comment will read in its final form. If the method parameters are clearly named and described in the narrative, then it isn't absolutely necessary to use the @param keyword. If there are multiple parameters, a non-void return value, or exceptions thrown, then those must be included in the javadoc. Feel free to use html tags in your comments, including <p>, <ol>, <tt>, <pre>, etc.

  /**
   * Submit a protocol event to the server. Customer and Supplier
   * subclasses may want to override this method. If they do, it
   * is required that they invoke <tt>super.submit(Message)</tt>.
   * @param newMessage  the Protocol Element to be submitted.
   */ 
  public void submit (ProtocolElement newMessage)
  {
    ...
  }

Parameter names will show up in the javadoc and must be descriptive, especially if they are primitive types.

public Employee remove(int d, double s)
        /* Huh? */ 
public Employee remove(int department, double severancePay)
        /* Ok */

Of course, for very generic functions, short names may be very appropriate.

public static void sort(int[] a)
     /* Ok */

Try to keep method bodies under half a page. Obviously, long if-else and try-catch chains will often exceed this limit. For method longer than about 10 lines, break up the presentation with indented running commentary. For an example of this style in an extremely long method, see edu.umn.cs.tac.oracle.eval.AllocationEvaluator.recomputeIfNecessary() (code in MinneTAC that writes out a linear program formulation).

Variables and constants

There is no need to define all variables at the beginning of a block of code. Code is usually easier to read if local variables are declared and initialized close to where they're used. Be careful, however, of declaring a variable inside an if or try and then expecting it to be visible outside. This often happens when you wrap some type of error handling around existing code.

It's bad form to define two variables on the same line

  int dimes = 0, nickels = 0; /* Don't */

Do not use magic numbers! A magic number is a numeric constant embedded in code, without a constant definition. Any number except 0, 1 and 2 is considered magic.

  if (p.getX() < 10) /* Don't */

Use static final instead.

  public static final double WINDOX_XMAX = 10; 
    . . .
    if (p.getX() < WINDOW_XMAX) /* Ok */

Public static final constants are often used as global state variables, or as selectors to methods. See the java.io package for a number of examples of this usage.

Control flow

The if statement

The nominal case should be in the if clause, and the exception case should be in the else clause.

Avoid the if...if...else trap. The code

  if ( ... ) 
    if ( ... ) ...; 
  else {
    ...; 
    ...; 
  }

will not do what the indentation level suggests, and it can take hours to find such a bug. Always use explicit braces {...} when dealing with if...if...else, and let your editor do the indentation for you. If your editor won't do that, then get a better editor.

  if ( ... ) {
    if ( ... ) ...; 
    else ...; 
  } /* {...} not strictly necessary here, but they keep you out of trouble */ 

  if ( ... ) {
    if ( ... ) ...; 
  } /* {...} are necessary here */ 
  else ...;

##The for statement Only use for loops when a variable runs from somewhere to somewhere with some constant increment/decrement.

  for (i = 0; i < a.length; i++) 
    print(a[i]);

Don't use a for loop for weird constructs such as

  for (a = a / 2; count < ITERATIONS; System.out.println(xnew)) 
    . . .
  /* Don't */

Make such a loop into a while loop. That way, the sequence of instructions is much clearer.

  a = a / 2; 
  while (count < ITERATIONS) { /* Ok */ 
     . . .
     System.out.println(xnew);
  }

A for loop makes sense with an Iterator:

  for (Iterator combinations = new CombinationIterator(usedOn);
       combinations.hasNext(); ) { /* Ok */
    Object[] temp = (Object[])combinations.next();
    . . .
  }

Exceptions

TBD

Naming

Strive for a good balance between descriptiveness and brevity in your naming. Don't be afraid to go back and fix up your names if you got them wrong; just make sure the javadoc gets regenerated afterward, and be careful about finding all the references. The master javadoc should gets updated once a day, and Eclipse will help you find the references if you do your renaming using the "refactor" capability.

When names refer to domain concepts, it is absolutely necessary to use domain terminology correctly. Misusing a domain term, and using the wrong term for a defined domain concept, are bugs.

Names should be reasonably descriptive. Use firstPlayer instead of fp. Don't drp yr vwls. Local variables that are fairly routine can be short (ch, i) as long as their scope is short enough that you can see the declaration from the furthest use, and they are really just boring holders for an input character, a loop counter etc. Do not use ctr, c, cntr, cnt, c2 for five counter variables in your function. Surely these variables all have a specific purpose and can be named to remind the reader of it (e.g. current, next, previous, result . . .).

Presentation

Use blank lines freely, but not gratuitously, to separate logically separate parts of a method. Avoid multiple blank lines within a method.

Separate related groups of methods by comments like //--------- all the way across the page.

Surround every binary operator with spaces.

  x1 = (-b - sqrt(b * b - 4 * a * c)) / (2 * a); /* Good */ 
  x1=(-b-sqrt(b*b-4*a*c))/(2*a);/*Bad*/

Leave a blank space after (and not before) each comma, semicolon, and keyword, but not after a method name in code. Put a blank space after the method name in a method declaration.

if (result == 0) 
foo(actions, billings[i]);

Every line must fit on 80 columns. Don't run your editor window clear across the screen. That makes life difficult for people who aren't blessed with such a big screen, or who must read the code on paper. If you must break a statement, add an indentation level for the continuation, and put the operator at the start of the second line:

  a[n] = .................................................. 
    + .................;

or better yet

  a[n] = .................................................. 
         + .................;

If this happens in an if or while, be sure to surround the body with braces, even if there is only one statement.

  if ( ......................................................... 
        && .................. 
        || .......... ) {
    . . .  
  }

If it wasn't for the braces, it would be hard to visually separate the continuation of the condition from the statement to be executed.

Braces

The opening and closing braces in a class declaration should be on lines by themselves, aligned below the opening keyword (normally the first column).

  public class OracleImpl
    implements Oracle, LogEnabled, Configurable, Serviceable, Initializable,
          ThreadSafe, StartOfGameListener
  {
    . . .
  }

The opening and closing braces in a method declaration should be on lines by themselves, aligned below the Method keyword.

  public Evaluation evaluate (Evaluable thing)
  {
    recomputeIfNecessary();
    return new Evaluation() {
        public Object getResult () {
          return (double[][])allocations.clone();
        }
      };
  }

Note that the anonymous class declaration here is treated differently from a named class, to make it look more like control flow.

In control flow, the opening brace should normally be on the same line as the control statement, and the closing brace on a line by itself, vertically aligned with the control statement.

    while (!done) {
      doSomething(); 
      done = moreToDo(); 
    }

Unstable layout

In general, try to get your editor set up to produce good results and go with it. If your editor isn't indenting right, fix the editor, don't futz with the code by hand. Most decent editors are customizable; if yours isn't, get a different one.

Some programmers take great pride in lining up certain columns in their code.

firstRecord      = readInt("First record:");
lastRecord       = readInt("Last record:");
cutoff           = readDouble("Cutoff:");

This is undeniably neat, but don't do it manually. The layout is not stable under change. A new variable name that is longer than the preallotted number of columns requires that you move all entries around.

firstRecord      = readInt("First record:");
lastRecord       = readInt("Last record:");
cutoff           = readDouble("Cutoff:");
marginalFudgeFactor = readDouble("Marginal Fudge Factor:");
⚠️ **GitHub.com Fallback** ⚠️