Setting a convention for code style

[antsouchlos]

As with the identifier names, it is long overdue to agree on a convention for code style. As with the naming convention, I'll start by proposing a convention. In order not to make this a never ending wall of text, I'll focus on the most important stuff.

Proposed code style convention

Cruly brackets

Curly brackets should always be on the same line, for everything (if statements, functions, classes, ...).

class SomeClass {
public:
    SomeClass(int i) {
        init(i);

        if (i < 10) {
            // ...
        } else if (i > 20) {
            // ...
        } else {
            // ...
        }
    }
private:
    void init(int i) {
        switch (i) {
            case 0:
                // ...
            case 1:
                // ...
            default:
                // ...
        }

        while (i > 0) {
            // ...
            --i;
        }
    }
};

Indentation

Use spaces, not tabs

I know, this one hurts me too. But hear me out.

It is possible align things much better using spaces, for example:

void do_smth(int first_arg, int second_arg,
             int third_arg, int fourth_arg,
             int fifth_arg, int sixth_atrg) {

    int a                = 0;
    int this_is_some_var = 3;
    char some_name       = 'c';
}

template<typename arg_t, typename first_t, typename rest_t,
         class = typename std::enable_if<std::is_base_of<argt_t, SomeType>>::type>
void do_smth2() {
    // ...
}

Also: with most IDEs it is possible to simply configure one tab to be equivalent to a certain number of spaces, so this shouldn't really change anything

Indentation width

One indentation level should be 4 spaces.

Statement alignment

Basically the example from above: If you have multiple assignments one after the other they should be aligned, same goes for function arguments and template parameters that don't fit on one line.

Columnt limit

The Columnt limit of a line should be at 80 (comments don't count), except in special cases (e.g. 20 declarations one after the other, each with a length of 81).

Namespaces

Namespace comments

The closing bracket of namespaces should have a comment to signal which namespace it belongs to:

namespace test {

// ...

} // namespace test

Indentation

There should be no extra indentation for stuff that is inside a namespace:

namespace test {

class SomeClass {
    // ...
};

void do_smth() {
    // ...
}

} // namespace test

Compact namespaces

For defining a namespace within a namespace, compact syntax should be used:

namespace oe { namespace detail {

// ...

}} // namespace oe::detail

[philsegeler]

Curly brackets

I already (mostly) do all of this, except in the } else if and } else cases. There i do:

if (i < 10) {
    // ...
} 
else if (i > 20) {
    // ...
} 
else {
            // ...
}

This looks better for me.

Indentation

Use spaces, not tabs

See *

Indentation width

See *

Statement alignment

See *

Column limit

I think 80 is too low. Your text editor should have line splitting built-in. Keeping it under 80 will be especially hard considering all the function namespaces and snake_case. Just take a look at src/Renderer/NRE_GPU_API.cpp and src/Renderer/NRE_RendererMain.cpp to understand what i mean. Many lines have more than 100 columns excluding indentation. I'd rather we have no such limit. This line for example:

nre::gpu::set_texture_format(this->colortexture, nre::gpu::RGB10_A2, nre::gpu::NEAREST, this->screen->resolution_x, this->screen->resolution_y, 0);

Namespaces

Namespace comments

No objections to that

Indentation

No way! Namespaces should have the normal 4 spaces as everything else.

Compact namespaces

Yes, if possible, but not always. Sometimes one may want to do something like this: (like for example in include/OE_API.h)

namespace A {

    int some_var1;

    namespace B {

        int some_var2;

    }; // namespace A::B

    int some_var3;

}; // namespace A

This looks to me far better than this monstrosity:

namespace A {

int some_var1;

}; // namespace A

namespace A { namespace B{

int some_var2;

}; // namespace A::B

namespace A {

int some_var3;

}; // namespace A

Namespaces should only have class and function declarations for the most part. We can live with extra indentations in header-only classes/functions i think.

• It would be useful to have a script that does all the formatting automatically instead of having to rewrite our code manually. I only accept the suggestions with See * if this script is available. :)

[antsouchlos]

Curly brackets

I accept your suggestion. We will put the else blocks of if statement on separate lines, all other curly brackets start on the same line.

Namespaces

Compact namespaces

I agree with your comment about compact namespaces. We should only use them where it makes sense, your counterexample is truly horrible.

Namespace indentation

Agreed, namespaces will have an indentation of 4 spaces.

See *

There is a wonderful tool called clang-format which does exactly that. I will commit a clang-format configuration file, once we have agreed on the code style.

Column limit

The only thing I cannot accept immediately is the columnt limit. We can set it to higher than 80, but I think the example line you posted is an eyesore.

How would you feel for example about something like the following?

nre::gpu::set_texture_format(this->colortexture, nre::gpu::RGB10_A2,
                             nre::gpu::NEAREST, this->screen->resolution_x,
                             this->screen->resolution_y, 0);

[philsegeler]

Curly brackets

Nice!

Namespaces

Compact namespaces

Nice!

Namespace indentation

Very nice!

See *

Great, implement that and put instructions in the wiki :)

Column limit

This would in theory be possible, but maybe not if something like this appears as often as it does. Maybe once or twice. Take a look at the NRE_RendererMain.cpp file and decide for yourself if you think modifying EVERYTHING with that format looks better (consider also it will at least double file lines). Many are just over 100 or 120. Maybe just increase it to like 128 (cause power of 2)? My example would then be:

nre::gpu::set_texture_format(this->colortexture, nre::gpu::RGB10_A2, nre::gpu::NEAREST, 
                             this->screen->resolution_x, this->screen->resolution_y, 0);

Then i also wouldn't have to change EVERYTHING, but only a few places here and there.

[antsouchlos]

I can live with a longer column limit. I'll set it to 128 (valid reason) and we can decide if we want to change it in the future. Anyrhing else? Otherwise I will close this issue.

[philsegeler]

No close it. Also please document the final agreed code style somewhere if possible.

[antsouchlos]

Reopening until the changed are implemented