Styleguide

From Picogen-doc

Those guides are not hard requirements. Read it once, and try to follow them roughly.

Contents

Basic Rules

Spartan Programming

See Spartan programming and this Codinghorror entry.

Especially the part about variables (thanks to Jeff Atwood for that excerpt):

  • 1. Minimize number of variables. Inline variables which are used only once. Take advantage of foreach loops.
  • 2. Minimize visibility of variables and other identifiers. Define variables at the smallest possible scope.
  • 3. Minimize accessibility of variables. Prefer the greater encapsulation of private variables.
  • 4. Minimize variability of variables. Strive to make variables final in Java and const in C++. Use annotations or restrictions whenever possible.
  • 5. Minimize lifetime of variables. Prefer ephemeral variables to longer lived ones. Avoid persistent variables such as files.
  • 6. Minimize names of variables. Short-lived, tightly scoped variables can use concise, terse names.
  • 7. Minimize use of array variables. Replace them with collections provided by your standard libraries.


Follow YAGNI and KISS

No enforcement, but generally your code shalt be easy to grasp, plus you shouldn't do work where it is not needed at the moment (if you need some functionality in future, you will anyway get your chance to implement it, if not, you saved a lot of time).

If some part of your code becomes a performance or overhead bottleneck (in any respect), then you can use more fancy algorithms.

Write Amalgam Friendly Code

The amalgam is all sourcecode files cat into a single one. This helps the compiler with whole-program-optimization and interprocedural analysis.

  • Write amalgam friendly. The amalgam file will be placed in redshift/bin, so all #include statements should fit to that. Upon amalgam build, the macro AMALGAM will be defined
  • Writing amalgam friendly is not always easy due to name clashes. But if such a name clash occurs, ask yourself:
    • Was the name I have given to a certain entity expressive enough? An extreme case is when you call a function simply "i()", which clearly expresses nothing.
    • If the function name was expressive enough, like "sin()", would it not have been better to just define it once and reuse it? E.g., place it in a header and declare it as an inline function.


External Dependencies

  • External dependencies shalt be kept to a minimum
  • If there is an external dependency, write a thin wrapper so that only a small piece of code is dependent (e.g. STATIC_ASSERT() wraps around BOOST_STATIC_ASSERT())


#define-Macros

No.



If still not convinced, present me how a class or function template won't do the job better, or how it would introduce too much kludge to not use a #define macro.


RAII

The ideal picogen object will only have constructors, const functions, and a destructor. It will have a mostly constant state. Mutable members are allowed, but with great care. Between construction and destruction, avoid changes in behaviour. Const member functions are not allowed to change behaviour from the perspective of the object's client.

TODO: Write reason


Use STATIC_ASSERT() Where Possible

Const Correctness

Generally, write const correct code (see also this article). As a rule of thumb: Make objects as const as possible. Make code as const as possible. But: Don't pass by const value, and don't return by const value; but but pass by const reference, and return by const reference.


Function Parameters

When writing a new function, prefer the form f(input)->output, meaning that the output of a function shall generally come from a return-value, apart from other things, this helps a lot with const correctness and RAII (also see this article):

float reciprocal (float x) {
    return 1.0f / x;
}

If you have multiple return values, use redshift::tuple (which comes from boost):

tuple<float,float> sincos (float x) {
    return make_tuple (sinf(x), cosf(x));
}

In general, avoid to use the same parameters as input and output:

void foo (float &x) { // please avoid
    x = x + x;
}

This might be a personal matter of taste, but I think it keeps conventions and code cleaner.

If you find out that using tuple<> yields a serious bottleneck, and you seriously have to pass input- and output-parameters, then write the function in the form f(input,output)->void, that is, input parameters first, output parameters last. This is also a Unix-convention (e.g. "mv old-filename new-filename"), a AT+T assembly language convention (e.g. "mov source, target") and also seems to be more natural for many instructions (e.g. "move chocolate from left to right" -> "move_chocolate(left, right)"). If you must decide to use that form, in general make the function void, or in other words, make it a procedure. Here is an example:

void foobar (int x, int y, int &frob) {
    frob = (x - y) * 5;
} 

Generally avoid the f(input,output)->void form, because in client code it might not be obvious which arguments are output arguments and which not (const correctness can help to some degree). There is also the alternative of passing pointers, so that in the client code all output parameters start with a '&' prefix, but that opens up loopholes again, because there is no promise that no nullptr will be passed.

Writing Style

Tabs And Whitespace

The size of a tab is 8 characters. A tab consists of 8 space characters. If you get into problems, see one of the other points about breaking up statemens.

Editor Width

Eighty (80) Columns. This is not only terminal friendly (though that argument seems deprecated for a graphical program like picogen), but you have a 99.9% guarantee that it prints fine on your printer of choice, in your editor of choice. Also, this rule makes it easy to place useful things on the left side and the right side of your non-spinnable widescreen (if any), like e.g. cheat sheets, documentation, or simply more editors. Additionaly, you automagically have an eye on whether your code gets too nested.

If unsure how to configure that in your editor of choice, just take the standard license header from any of the source-files, which is exactly 80 columns wide.

Braces

Breaking Up Statements

Naming Conventions

Naming In General

Type pre/post-fixes

Generally, don't give your entity a type-dependent name, we're not MSVC 6.0 developers.

But in some (probably lower level) situations, we have several variables which express the same things, but are of different type. E.g.:

for (int u=0; u<128; ++u) {
         const float uf = static_cast<float> (u);
}

In such cases, as seen in the example, give your primary variable ('u' in the example) a normal, "generic" name, and to the type-changed "aliases" apply a simple postfix. There's no rule-of-thumb, just use something more-or-less intuitive. The primary purpose of the postfix is not to tell the type, but to give an entity a similar but distinct id.


Canonical Names

We do not enforce these, but highly encourage the use of them.

code usage example
filename, fooFilename
Used for names of files. The form fooFilename is used whenever the meaning must be more specialized (e.g. when you multiple filenames in handy). Simply "file" is not enough, as it could also imply a FILE* handle, or similar.
std::string const filename ("foo");
file, fooFile
Used for files (not to describe the name of a file!).
ifstream inFile  ("source.xml", ios::in);
ofstream outFile ("dump.txt", ios::out);
fin, fout
Used for input/output files (not to describe the name of a file!).
ifstream fin  ("source.xml", ios::in);
ofstream fout ("dump.txt",   ios::out);
tmp, tmpfoo, tmpFoobar
Use this only in contexts of very small scope to describe a temporary variable. May hold anything.
void foo () {
  int tmp = x + y;
  int x = tmp * 4;
  float tmpf = 0.333f;
  float tmp0 = 0.444f;
  float tmp1 = 0.555f;
}
for (int i=...)
for (int u=...)
for (iterator it=...)
while (!done)
Self explaining: Those are the names for loop variables.

Optimization

In general, you ain't gonna need it.

No Bit Twiddling

Don't use any bitwise operations. Except for the case you provide a conditionally compiled, portable alternative.

TODO: which are the conditions?


Prefer being expressive instead of doing by hand what the compiler does already

Write

int i = j * 2;

and not

int i = j << 1;

. Any modern compiler will do the job for you already. And sometimes it will even do better. E.g. it will decompose

int i = j * 3;

into

int i = j<<1 + j; // compiler does that for you!

. But there is an exception to this: Conformant compilers are not allowed to do certain optimizations on IEEE floating point numbers, like reordering operands in many cases (and thus staying in the way of constant propagation), or replacing certain divisions by a number with three multiplications with the inverse number. GCC can be forced to do this, but we shouldn't rely on it.

So, instead of

float const
        r = 4 / f,
        g = 0.5 / f,
        float const b = g / f
;

write

float const
         inv = 1.0f / f,
         r = 4 * inv,
         g = 0.5 * inv,
         b = g * inv
;

.

And if you have

float const f = 2.0 + x + 1.0;

do the constant folding yourself and write the original formula in a comment:

float const f = 3.0 + x; // == 2 + x + 1

.

Example

Rather sensefree code, but this pretty much summarizes some of the above rules.

       // 1 tab = 8 single space characters.
       // Also, width of line is 80 chars for proper printing and to just
       // have some upper bound :)
 
       #include <system-headers>             // e.g. sys/mman.h
       #include <std-c++-headers>            // e.g. iostream
       #include <intermediate-level-headers> // e.g. QT, SDL, OpenGL
       #include <highest-level-headers>      // e.g. MeatballEngine
 
       namespace {
 
       // No indendation inside namespace for implementation files.
       // In declaration files, I indent inside a namespace.
 
       class FooClass : FooBase {
       public:
               FooClass () : FooBase() {}
               ~FooClass() {
                       do {
                               for (int i=0; i<5; ++i) {
                                       thisAndThat();
                               }
 
                               // If we have an empty loop,
                               // make the semicolon a bit outstanding.
                               // (alt.: put in next line and indent)
                               for (int i=0; i<5; ++i) ;
 
                               if (callWithoutArgs()) {
                                       anotherCallWithArgs (5, 66, tuple<T,X>());
                               }
                       } while (1 == condition);
               }
       };
 
 
 
       // three blank lines between definitions
       int faculty (int i) {
               return i<2
                      ? 1
                      : i*faculty (i-1);
       }
 
 
 
       void tooManyArgsToFitOnALine (
               Vector const & a,
               Vector const & b,
               Vector const & c
       ) {
               const Vector cross = a * b;
               const real dot = a * b;
       }
 
 
 
       namespace boo { namespace noo { namespace moo {
       class ForwardDeclaredClass;
       enum foonum_t {
               alpha, bravo, charlie
       };
       } } }
 
 
 
       int main () {
               using std::cout; // only if used often
 
               FooClass fooInstance;
 
               // Everything as const as possible. See section "Spartan Programming".
               // I frown upon 'using namespace'.
               const boo::noo::moo::foonum_t pipapo = alpha;
               const float
                       alpha = 0.5f,
                       bravo = alpha * 0.5f + sqrt (powf(alpha,alpha)),
                       charlie = pi
               ;
 
               int i;
               std::cin >> i;
 
               switch (pipapo) {
               case alpha: // fallthrough     << tell about intention
               case bravo:
                       foo();
                       bar (pipapo);
                       cout << "Heh.\n"; // No unnecessary flushing.
                       cout << "Meh.\n";
                       break;
               case charlie:
                       break;
               // No default: to get warnings from the compiler for missing
               // enum value.
               };
 
               cout << std::flush;
       }