Miniature/Policies

(Advanced coding style)
(Advanced coding style)
 
(One intermediate revision not shown)
Line 27: Line 27:
== Naming conventions ==
== Naming conventions ==
-
* Methods *do* seomthing, that's why their names should always contain a verb (slots and signals count as methods, too).
+
* Methods *do* something, that's why their names should always contain a verb (slots and signals count as methods, too).
* typedef'd types *might* hint at their composed types (to emphasize their intended use), but avoid directly including the type information in the name:
* typedef'd types *might* hint at their composed types (to emphasize their intended use), but avoid directly including the type information in the name:
Line 100: Line 100:
* Assign 0 to pointers after deletion (or use QPointers).
* Assign 0 to pointers after deletion (or use QPointers).
* Always initialize type primitives (e.g., pointers) in the constructor's initializer list.
* Always initialize type primitives (e.g., pointers) in the constructor's initializer list.
 +
* Don't use forward declaration unless necessary. It is not your task to optimize the compiler. Also, Qt Creator gets confused by them.
== How to keep code testable (and hopefully, mostly bugfree) ==
== How to keep code testable (and hopefully, mostly bugfree) ==

Latest revision as of 23:34, 10 May 2010

This is a list of things we agreed upon in the Miniature team. TODO: List the items that are *not* fully agreed upon yet!!

Contents

[edit] License

Miniature will use the GPL 2 (or later) for source and header files. For art contribution, Miniature will use the CC-by-sa.

[edit] Coding style

We do not need project-wide consistency, but be at least consistent inside a module and/or file.

Note: the current code violates this style. We will fix it.

We mostly try to follow the offical Qt Coding Guidelines, but with some exceptions. follow mikhas' personal coding style for Qt:

  • We use whitespaces over tabs.
  • Max. length of a source code line is 120 characters.
  • We use a Miniature namespace for all our components, and all our types(classes + typedefs) start with M.
    • Exception: value types (containers, flags) start with a lowercase m, for example mMoveFlags or mHalfMove. Value types are useful when you need copy construction and copy assignment. In general though, objects are *not* copyable! MPosition is the biggest violation to this rule, feel free to fix it!
  • Types are in MCamelCase, unless they are mValueTypes (see above).
  • Method names are in mixedCase (= lowerCamelCase).
  • Braces after newlines.
  • Variable names should be visually different from types (even if you use an editor w/o syntax highlighting), there_fore they have under_scores, no caps, no lower_Camel_Case.
  • Member variables should be visible, even if your editor does not highlight member variables for you. That's why we prefix them with m_*.
  • We use no_keywords, for now. We want to avoid those function-style macros.
  • Impl files have a .cc extensions, header files use .h
  • Use Qt containers over std containers.

[edit] Naming conventions

  • Methods *do* something, that's why their names should always contain a verb (slots and signals count as methods, too).
  • typedef'd types *might* hint at their composed types (to emphasize their intended use), but avoid directly including the type information in the name:
 typedef QSharedPointer<SomeClass> SomeSharedClass; // Good! Sharing a resource is a concept.
 typedef QSharedPointer<SomeClass> SomeClassPtr; // Bad! What is a "SomeClassPtr *some_variable" now?
 typedef QList<SomeClass> SomeClassList; // still OK, since "List" is a concept, too.
  • Avoid hungarian variable names.

[edit] Advanced coding style

  • Signal and slot connections should be easy to read, as they are a constant source of bugs:
 connect(some_signal_source, SIGNAL(theSignalWasEmitted()),
         some_receiver,      SLOT(onTheSignalWasEmitted()),
         Qt::UniqueConnection);
    • Details:
      • slots should follow the signal's name, if possible (slot prefix: on*).
      • Indentation is one line for the source/signal, one for the receiver/slot and another one for the connection type.
      • SIGNAL and SLOT should start in the same column (together with the slot's name prefix it will neatly align the parameters etc.).
      • Use normalized signal/slot signatures (no const keyword, no &).
      • Use Qt::UniqueConnection for the connection type (it's *not* the default connection type), since most of the time, it is exactly what you want. A double connection between the same source/receiver and SIGNAL/SLOT is often a bug, and this can lead to weird behaviour if the same connection code is called over and over again, leading to what I call "connection leaks". With Qt::UniqueConnection, you clearly state the intent of the connection type, while also preventing those connection leaks.
  • Method parameters in header files (*not* needed in impl files) should be easy to read:
 class SomeClass
 {
     void someMethod(SomeType arg1,
                     SomeOtherType arg2,
                     ...);
    • Details:
      • Move each argument to a new line. This serves two goals: header files will be easier to scan (for humans), and you'll spot those ugly methods that scream "refactor me!" faster because they have more than 3 arguments. Also, because it *is* ugly, you might just go and refactor it, which will help everyone.
  • Use const refs for arguments when possible, even if the type is COW (copy-on-write, for example QString).
  • Conditionals
    • One line before and after conditional blocks, to visually separate it from the surrounding code.
    • It is OK for single-line conditional blocks to omit the braces (because of the extra newlines).
    • && and || belong on the next line, if the condition needs to be wrapped (indented by 4 spaces). Put braces around complex conditions, to logically group them together (and to prevent operator precendence bugs, too):
 if ((something && somethingElse)
      || orPerhapsThis)
     ...
    • Details:
      • Conditionals often contain bugs. Especially when the surrounding code gets refactored, those bugs simply slip in.
      • If a conditional needs to be wrapped, it starts to look ugly. Good! It might lead you to refactor the code, moving the condition itself into a predicate function (which has the nice effect to serve as a comment, if it has a good name).
      • Nested conditionals quickly start to look ugly with all those line separators, but you probably guessed it already: It's just another sign that this code might need some cleanup! Nested conditionals should be used rarely or hidden within dedicated methods.
  • One space after keywords that could be mistaken as functions (if, for, ...).
  • If you store a pointer to an object over which you do not intend to take ownership, then wrap this pointer in a QPointer:
 class SomeOtherClass
 {
     QPointer<SomeType> m_member;
 }
    • Details:
      • QPointers keep track of object deletion and set all referring QPointers to zero, preventing dangling pointers. Especially when using signals & slots, this is an often (and sometimes hard to spot!) source of bugs. With QPointers, you can wrap your code with conditional guards like so:
 if (m_member)
 {
     // Do sth with m_member object.
     // This block *never* gets executed even if m_member was deleted elsewhere.
 }
  • Assign 0 to pointers after deletion (or use QPointers).
  • Always initialize type primitives (e.g., pointers) in the constructor's initializer list.
  • Don't use forward declaration unless necessary. It is not your task to optimize the compiler. Also, Qt Creator gets confused by them.

[edit] How to keep code testable (and hopefully, mostly bugfree)

We want to have testable code, therefore we also want to follow some advanced coding standards.

For that, I will try to list some really really useful advice from this document: http://www.research.att.com/~bs/JSF-AV-rules.pdf, a coding style document for C++. Bjarne Stroustrup worked on that one, yes.

  • Keep methods at a reasonable length. In general: If it doesn't fit on a screen page it's probably too long. More precise: Every method longer than 200 lines of code (including comments, yes) is too long.
  • Within a method, avoid a cyclomatic complexity higher than 20 (see Appendix A regarding AV Rule 3, pp. 65, "Cyclomatic complexity measures the amount of decision logic in a single software module.", and the example).
  • Avoid cyclic dependencies between classes/modules (Rationale: it's mostly an indicator for layer violations).