Shotwell Vala Coding Conventions

Shotwell is written in Vala. The following guidelines are shaped toward describing our preferred coding style for Vala.

The goals of these coding conventions are consistency and readability for a team of coders, in the service of producing quality code. There are few hard-and-fast rules here. Use good judgment when stretching their boundaries or violating them outright. Ask if you’re unsure.

Formatting

Ordering

The members of each class should appear in the following order:

  1. consts
  2. enums
  3. inner classes
  4. static properties
  5. static fields (blocked in this order: public / protected / private)
  6. instance properties
  7. instance fields (blocked in this order: public / protected / private)
  8. signals
  9. constructors
  10. destructor (optional)
  11. methods

Basics

  • Source lines may be up to 100 characters long. (You can configure gedit, vim or builder to display a margin characters; that may help you follow this convention.)
  • Functions or methods longer than about 50 lines are discouraged. This is about the amount of code that can be easily viewed on a tall computer monitor. A function or method longer than 100 lines should certainly be split up if reasonably possible.
  • Variables should be declared at the point where they are first needed rather than at the top of a block or function. If the variable is declared when needed, it often can be initialized with a useful value. Vala initializes all variables, but they still should be initialized explicitly unless delayed due to a conditional.

Right: 

int x = get_x();
int y = get_y();

int width, height;
if (is_tall()) {
    width = 0;
    height = 100;
} else {
    width = 100;
    height = 0;
}

  • Use this only when needed for disambiguation. In general, it should only be needed in constructors and setters.

Right: 

class Pair {
    int x;
    int y;

    public Pair(int x, int y) {
        this.x = x; // "this" needed for disambiguation
        this.y = y;
    }

    public void swap() {
        int t = x; // no "this"" here or on following lines
        x = y;
        y = t;
    }
}

  • Use named constructors. Avoid construct blocks when possible.

  • Don’t use the using statement. Explicitly preface each symbol with its namespace name. For example, Gtk.Window rather than just Window.

    • Symbols within the code's namespace can use Vala's implicit namespace rules.

    • Note that Vala imports the GLib namespace automatically. In our code, the GLib prefix is optional when referring to a symbol in that namespace; either GLib.File or simply File is just fine.

Naming

  • Follow Vala conventions for naming identifiers.

    • Variables and class fields are lowercase_with_underscores.

    • Namespaces and class names are CamelCased.

  • Source filenames are determined by each project; our current scheme in Geary is to use namespace-class-name-with-dashes.vala. To avoid long filenames, use the namespace above the class name only. Thus:

    • Geary.EmailIdentifier is located in geary-email-identifier.vala

    • Geary.Imap.MailboxSpecifier is in imap-mailbox-specifier.vala.

  • In variable names, we discourage several-character abbreviations. It’s preferable to use a full word.

Right: 

Gtk.Window window;

Discouraged: 

Gtk.Window w;

  • Single-character variable names (e.g. w) are allowed for local variables only where its use is obvious (i and j for counters, x and y for coordinates, etc.)

Braces

  • An opening brace appears on the same line as preceding code.

Right: 

void foo() {
    for (int x = 0 ; x < 10 ; ++x) {
        ...
    }
}

Wrong: 

void foo()
{
    for (int x = 0 ; x < 10 ; ++x)
    {
        ...
    }
}

  • A brace preceding or following an else keyword appears on the same line as the else. If a single if-else if-else block is braced, all should be braced.

Right: 

if (x == 4) {
    ...
} else {
    ...
}

Wrong: 

if (x == 4) {
    ...
} else ++y;

if (x == 4) {
    ...
} else
    ++y;

if (x == 4) {
    ...
}
else {
    ...
}

  • A brace preceding a catch keyword appears on the same line as the catch.

Right: 

try {
    ...
} catch (Error e) {
    ...
}

  • A value in a return statement is not parenthesized.

Right: 

return x;

Right: 

return a + b;

  • If the body of an if, for, while or similar statement consists of a single statement, the statement may optionally be surrounded by braces. The statement should always be on the following line.

Right: 

if (x == 3)
   ++x;

if (x == 3) {
   ++x;
}

Wrong: 

if (x == 3) ++x;

Spacing

  • Spaces, not tabs, for indentation.
  • Indent each nesting level by 4 characters.
  • A blank line should be padded to the “natural” indentation level of the surrounding lines.
  • There should be no successive blank lines in code.
  • If a namespace block encloses an entire source file, then definitions inside the namespace are not indented. If a namespace block encloses only part of a file, definitions inside it are indented.
    • In general, it's best to enclose the entire source file in a single namespace. If code needs to be outside the namespace, consider using a different source file.
  • Use spaces around binary operators.

Right: 

a = x + y;

if (x == y)
    ...

Wrong: 

a = x+y;

if (x==y)
    ...

  • There should be no space between the ++ or — operator and a variable name.

Right: 

++i;
x = j++;

Wrong: 

++ i;
x = j ++;

  • Unary operators such as ! and ~ are not followed by a space.

Right: 

left = !right;
stopped = !process.is_running();

Wrong: 

left = ! right;
stopped = ! process.is_running();

  • In an array declaration and access, there should be no space between the array name and the opening left bracket.

Right: 

int i[10];
x = i[4];

Wrong: 

int i [10];
x = i [4];

  • In a type cast, there is a space following the right parenthesis.

Right: 

int i = (int) f;

Wrong: 

int i = (int)f;

* Keywords such as if, for, and while are followed by a space, then a left parenthesis.

Right: 

if (x == 4)
    return;

Wrong: 

if(x == 4)
    return;

if( x == 4 )
    return;

  • In a variable declaration, a single space appears between the type and variable name.

Right: 

int count;
bool is_complete;
Window main_window;

Wrong: 

int    count;
bool   is_complete;
Window main_window;

  • In a method call or declaration there is no space between the method name and following left parenthesis, and no space appears after the left parenthesis or before the right parenthesis.

Right: 

int add(int a, int b) {
    ...
}

int i = add(4, 5);

Wrong: 

int add (int a, int b) {
    ...
}

int i = add ( 4, 5 );

  • A single blank line should appear between each pair of functions or methods.
  • A single blank line should appear between a field declaration and a following constructor or method.
  • There should be no blank line after an opening brace or before a closing brace. An exception is after the opening brace in a namespace statement, and before its closing brace: 

namespace City {

class House {
    int xyz;
    ...
}

}

  • Continued lines receive a single additional indent. Do not indent to line up method arguments with the line above them.

Comments

  • Comments are encouraged. However, if code requires comments, consider restructuring your code.

  • Valadocs are strongly encouraged in library code (i.e. the Geary engine). See section on Valadocs, below, for detailed information.

  • Checking in commented-out code is strongly discouraged.

  • In a single-line comment, the opening characters // should be preceded by one or more spaces, and should be followed by a single space. Trailing comments are discouraged and should be used sparingly. Comments preceding code lines are preferred:

Acceptable: 

int count; // number of widgets in the conglomeration

Better: 

// number of widgets in the conglomeration
int count;

Wrong: 

int count; //number of widgets in the conglomeration
int count;// number of widgets in the conglomeration

Language Features

Casting

  • Vala does implicit type-checking of method arguments when the --enable-checking compile flag is set. Both Geary and Shotwell use this flag.

  • There are three operators related to casting in Vala: is, as, and C-style casting (i.e. Object m = (Object) n;). Their proper use is important.

  • is and as can only be used with non-compact classes. Casting of native types (i.e. int, char, etc.) can only be done with C-style casts.

  • The is operator returns true if the object inherits from or implements the specified type. Note that is also returns true if the object implements a particular interface: 

if (obj is Gtk.Widget)
    // do something

  • The is operator can also be used to check the domain and error code of an Error (exception), or merely its domain: 

try {
    ...
} catch (Error err) {
    bool is_cancelled = err is IOError.CANCELLED;
    bool is_ioerror = err is IOError;
}

  • The as operator is a “safe” cast in that it returns null if the object is not of the specified type (or does not implement the specified interface). Because of this behavior, the returned type should be marked nullable and tested for null before use: 

Gtk.Widget? widget = obj as Gtk.Widget;
if (widget != null)
    widget.show_all();

Wrong: 

Gtk.Widget widget = obj as Widget;
widget.show_all();

  • C-style casting is not as “safe” as the as operator. It does perform GType checking before returning and will dump a warning to the console and return null when a bad cast occurs. However, see the next point for why a nullable type is not used in a C-style cast.

Right: 

Gtk.Widget widget = (Gtk.Widget) obj;
widget.show_all();

Wrong: 

Gtk.Widget widget = (Gtk.Widget) obj;
if (widget != null)
    widget.show_all();

  • When casting an object, use as for a test-and-use pattern and use C-style casting when the object should be of a certain type (i.e. no test is required). In other words, use a C-style cast when expecting non-null, hence a nullable type is not required.

  • assert() is often used to check parameters or return values for typeness. Do not use assert with the is operator prior to an as operation or a C-style cast; both defeat the purpose of Vala’s casting model.

Wrong: 

assert(obj is Gtk.Widget);
Gtk.Widget widget = (Gtk.Widget) obj;

  • Non-compact classes in Vala also have the get_class method which returns GType information, which can then use used for type checking and testing. This should only be used in specific circumstances where the Vala operators are insufficient or unavailable. The typeof operator is also useful here.

switch-case

  • In a switch statement, a break should be indented at the same level as its corresponding case label. A return or continue should be indented as regular code.

  • There should be a blank line between each case block.
  • A case block should not be braced.
  • An empty case block should contain a comment explaining why it's empty; "do nothing" is sufficient to indicate that it's presence is merely to allow flow of control through the switch statement without any side-effects.

Right: 

switch (suit) {
    case Suit.SPADES:
        do_something();
    break;

    case Suit.HEARTS:
    case Suit.DIAMONDS:
        do_something_else();
        return;

    default:
        // do nothing for other suits
    break;
}

Wrong: 

switch (suit) {
    case Suit.SPADES:
        do_something();
        break;

    case Suit.HEARTS:
        do_something();
    return;
    default:
    break;
}

  • Note that Vala doesn’t allow falling out of a case block without a jump statement of some kind, even if it’s the last block in the switch.

Signals

  • Signals should be declared like methods, i.e. a blank line between each declaration.

Right: 

public signal void plain_signal();

public virtual signal void virtual_signal() {
}

public signal void another_plain_signal();

Wrong: 

public signal void plain_signal();
public virtual signal void virtual_signal() {}
public signal void another_plain_signal();

  • Signal handler methods should have descriptive names beginning with on_.

Right: 

search_menu.activate.connect(on_search_menu_activated);

  • Signals should only be directly fired by the class itself. In rare cases it’s okay for a subclass to fire a signal. Having code external to the class directly fire its signals suggests a design flaw.

  • Our projects often use notify_ wrappers for firing signals. These are specific to core classes that use signals extensively and are presumed will be subclassed, and the subclasses will need to monitors signals for their own synchronization purposes. In this case, the notify wrappers are always protected virtual methods (as they should only be called by the base class and overridden by its children).

Right: 

public signal void count_changed(int new_count);

protected virtual void notify_count_changed(int new_count) {
count_changed(new_count);
}

This allows for a subclass to override notify_count_changed and update its internal state before or after the signal has fired.

Properties

  • Properties should be constrained to simple getter/setters for public values.

    • If the value is not set in the constructor, it should be set using the default keyword, even if the default is the natural default for that value (i.e. 0, 0.0, or null).

    • If a simple property is only a getter, mark it as private set.

    • Avoid construct properties.

  • Like methods and signals, leave a single blank line between each property declaration.
  • If a property has no code blocks, its modifiers should be listed on a single line. If it has any code blocks, they should be listed as indented separate code blocks:

Right: 

public int apple_count { get; private set; } // default set in constructor

public int orange_count { get; set; default = 0 }

public int banana_count {
    get {
        return (banana_list != null) ? banana_list.size : 0;
    }
}

private Gee.List<Banana>? banana_list = null;

  • A property code block should be no more than 2 - 3 lines of code. Anything longer strongly suggests a hand-coded getter and/or setter is required.

    • GObject provides the "notify" signal for detecting when a property has changed; Vala’s syntax is notify["property-name"].)

  • Shotwell uses static get_instance() methods for singletons. Geary uses static getter properties named instance for singletons.

Example (Geary): 

private static Singleton? _instance = null;
public static Singleton instance {
    get {
        return (_instance != null) ? _instance : _instance = new Singleton();
    }
}
  • Note that when a property is “shadowing” a private field, the private field is listed immediately before (with no whitespace) the property itself (as in the above example). Otherwise private member instance variables should be listed after the property declarations.

Closures (anonymous methods)

  • In general, we discourage using closures and historically have avoided them (for legacy compiler reasons). We still prefer to avoid them. However, there are situations where they make sense. Use judgement and ask if you have a question.
  • Closures should be no more than 2 - 3 lines of code; even then, more than one line is discouraged without justification. Anything longer strongly suggests a separate method should be used.
    • However, if a system by design uses delegates and is more convenient and clean to take advantage of Vala's closures, then so be it. One example is Geary's Db module, where closures are passed to the database transaction methods and are called by a separate thread.
  • Use full parameter names in the closure declaration, as though it was a “real” method. If the parameters are not required, do not declare them.
  • If the closure is a single line of code, write it that way. If it requires multiple statements, list them as though a “real” method.

Right: 

window.destroy.connect(() => { debug("destroyed"); });
window.draw.connect((ctx) => {
    debug("draw");
    ctx.clear();
});

Wrong: 

window.destroy.connect((window_param) => {debug("destroyed");});
window.draw.connect((ctx) => {
    debug("draw");
});

Default arguments

Vala supports default arguments. As convenient as this feature can be, it can also introduce bugs.

  • Public methods can use default arguments when they make sense. Private methods should not use default arguments.

  • One exception to this is public methods in a private or internal class. Since those methods are public only to other classes in the namespace, they are treated as “private” in terms of default arguments (i.e. should not have default arguments).

  • Cancellables are an important use of default arguments, especially in Geary. Cancellable defaults to null throughout GIO as an accepted practice. As with the above guideline, if the method is public, it can default to null; otherwise, it should not.

Other language features

  • Using the ternary operator in place of simple if-else blocks is strongly encouraged.
  • Remove any unnecessary automatic variables when possible.

  • Remove or revise less-than-useful comments.

Valadoc

  • We’re making a concerted effort to document the Geary engine with Valadoc. Similar to Javadoc, Valadoc uses specially-formatted comments that can be parsed by tools to auto-generate documentation.

  • A reference guide for Valadoc markup can be found at http://valadoc.org/#!wiki=markup

  • Going forward, all public classes, methods, and namespaces in Geary’s src/engine directory should be Valadoc'ed.
    • Internal and private classes do not need it, but of course comments are always encouraged.
    • Public methods of internal/private classes do not require Valadoc.

* If a symbol is an override or an interface implementation of a symbol documented in its parent class or interface, use the inheritDoc taglet: 

/**
 * {@inheritDoc}
 */

Additional notes about the symbol should follow the taglet.

  • Other taglets are optional. Use discretion. The see taglet can be very useful.

  • Valadocs for Geary can be produced with this command in the source code: 

$ make valadoc

This will generate HTML in the valadoc/ directory.

Basics

* A protypical Valadoc for all types of code objects looks like this: 

/**
 * A single line summary with a period.
 *
 * Longer (and, for simple code objects, optional) summary explaining the class/method/property in greater
 * detail. Like code lines, the summary lines should not exceed 100 characters per line.
 */

  • The summary line is required. The longer summary is optional, but only for the simplest of methods (usually properties).

  • Use Valadoc markup whenever appropriate. The first reference to a symbol should always be linked (i.e. @link Geary.EmailIdentifier), even if the symbol is the current class. The reason for this is that the current Valadoc generator doesn't automatically generate links to the symbol's container.

* Use HTML links whenever appropriate if the code is based off a specification or foreign API.

Classes

Each class should have a summary Valadoc. Place a single blank line between its Valadoc and the class declaration. 

/** 
 * A representation of the IMAP APPEND command.
 *
 * See [[http://tools.ietf.org/html/rfc3501#section-6.3.11]] 
 */
public class Geary.Imap.AppendCommand : Command {

Other symbols

  • Methods, properties, enums, and consts should also have Valadoc. There should be no blank line between the Valadoc and the declaration. 

/**
 * Returns the direction of the overhead oscillating fnortiner.
 */
public bool clockwise { get; private set; }
  • To document enums: 

public class Fnortiner {
    /**
     * Describes the {@link Fnortiner}’s position in the {@link Gizmo}.
     */
    public enum Position {
        /**
         * Indicates the {@link Fnortiner} is upside-down.
         */
        NORMAL,
        /**
         * Indicates the {@link Fnortiner} is inverted.
         */
        REVERSED
    }
}

Note how the containing class is linked in the Valadoc.

Apps/Shotwell/CodingConventions (last edited 2016-11-11 17:55:19 by JensGeorg)