Styleguide

From Picogen-doc

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

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

• KISS
• YAGNI

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.

Function calls in expressions

Except for where there is only a single call, this should in general be avoided, except it can be trivially deduced that the passed expressions are pure.

The justification for this is that a) it often makes code more readable, especially when functions accept multiple arguments, and b) the C++ programming language does not define a sequence point between parameters.

Consider this example:

void foo (int a, int b) { ... }
int main () {
    foo (rand(), rand());
}

Now, the C++ standard does not dictate which of the two calls to rand() is evaluated first. It could be that the net-result of above is

    foo(42, 1024);

but it could also perfectly be that it is

    foo(1024, 42);

because the order of evaluation was flipped, which is, again, perfectly allowed behaviour as per the standard.

You realise that the program's result may vary depending on this. Two reasons why this is relevant for picogen:

• The output of picogen should be the same on each supported platform for the same input, pixel-wise that is. This enables users to run picogen on several machines with different operating systems, using different random seeds, and later merging them together again without the fear of doubly rendered parts
• A lot of content is procedurally generated, and following the butterfly effect, non fully specified behaviour might have catastrophic effects and change or even destroy whole landscapes (and later grand universes, of course)

So, the solution to our example problem is to introduce a sequence point between the two invocations of rand() by hand, which is fairly trivial:

    const int a = rand(); // ";" is a sequence point
    const int b = rand();
    foo (a, b);

or

    const int a = rand(),
              b = rand();
    foo (a, b);

The latter works because a chain declaration is defined to be equivalent to the unchained version.

Results are consistent accross compilers/OSes now.

But also be careful in more complex expressions like

    int x = foo(bar()) + 3 * bar();

bar() might be a non-pure function like rand(). Again, split this up.

And even in some seemingly simple situations, one might face problems:

    struct Foo { 
        int operator() () const {
            ...
        }
    }; // functor
    int main () {
        Foo alpha, bravo;
        int charlie = alpha() + bravo();
    }

This does look okay, operator() is const at last. BUT: The relevant point is that even const functors and const member functions may depend on mutable state. Let's unfold the ...'s above:

    struct Foo { 
        int operator() () const {
            return rand();
        }
    }; // functor
    int main () {
        Foo alpha, bravo;
        int charlie = alpha() + bravo();
    }

So simple it goes down the drainage.

And it doesn't get easier: Even when there is no explicit function call apparent, there might still be one under the hood; just think of overloading conversion operators. Anyways, it is discouraged to write conversions that depend on class mutable state or any global state, thus this is ignored.

Rule of thumb: Always assign the result of a function call to a local variable.

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.

filename, fooFilename

std::string const filename ("foo");

file, fooFile

ifstream inFile  ("source.xml", ios::in);
ofstream outFile ("dump.txt", ios::out);

fin, fout

ifstream fin  ("source.xml", ios::in);
ofstream fout ("dump.txt",   ios::out);

tmp, tmpfoo, tmpFoobar

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)

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;
       }