Saphirac / ft_irc

MIT License
1 stars 1 forks source link

Define our own coding norm #2

Open Saphirac opened 7 months ago

Saphirac commented 7 months ago

In order to make reviews easier, we should discuss and decide about a common coding norm. At least, the following rules should be discussed:

JonathanDUFOUR commented 7 months ago

I agree, and so I suggest:

class ClassB { signed char a; signed short b; signed c; signed long d; signed long long e; unsigned char f; unsigned short g; unsigned h; unsigned long i; unsigned long long j; size_t k; bool l; }

struct StructA { char a; short b; int c; }

struct StructB { signed char a; signed short b; signed c; signed long d; signed long long e; unsigned char f; unsigned short g; unsigned h; unsigned long i; unsigned long long j; size_t k; bool l; }

ClassA const ca = ClassA { '', 12345, 67890 }; ClassB const cb = ClassB { '', 12345, 67890, ... }; StructA const sa = StructA { '', 12345, 67890 }; StructB const sb = StructB { '', 12345, 67890, ... };


- where do we declare and define functions: except for templates, I suggest always defining functions in source files separated from header files. The only exception to this would be for the template function's definition.
- naming convention: I think that names that are the most explicit possible are better for a fast understanding of what the code does, even if the names become verbose.
Saphirac commented 7 months ago

I do agree for the type of cases, but I'd like the braces better like this :

void () { if () {

... } else if () { ... } else { ... } for (; ; ) { ... } } as i've gotten used to the 42 norm and would like to continue as such. I feel like it's also more proper in a bigger file. I also have a new issue, should we rather declare our variable all in the beginning of the function (if possible) or as we using them ? Can we declare them and assign them in the same line ? ex : int a = new_value_a(x); I think it's practical to do this but maybe less clear.
JonathanDUFOUR commented 7 months ago

The brace style that you mentioned is okay for me, let's use it for our norm then. Concerning the declaration of constants and variables, I would prefer to declare them as late as possible but always follow their declaration with an empty newline. I also suggest declaring the indexes and the iterators that we would use only in a for loop inside of the <start> statement of this same for loop. And about the question of one-line declaration + assignation, I would prefer us to do it as long as it is not objectively preferable not to assign it on the same line as their declaration.

I also had an additional request that concerns the code documentation: I suggest that every function must have its dedicated comment right above it, to explain what the function does, what its parameters refer to, what it return, etc... I suggest using the doxygen comments for that purpose.

Another point that we haven't mentioned yet, but that is still important is: which maximum length do we allow for a single line? I suggest 100, but I may also accept more up to 120.

Saphirac commented 7 months ago

I'm not sure that I understand exactly what you meant for the variable declaration and - would like some examples.

The doxygen comments are a great idea. I already began to do it on some complex functions. However for some function it may not be useful such as :

(It's just an example we haven't got a real function like that)

bool IsCharacterOkay(char c)
{
    If (c != whatever)
       return (false);
    return wrong;
}

Because it's pretty self explanatory. I don't really see the use for a precise limit for line length as it's quite hard to check, we should use our common sense and separate it the best we could for functional use and comprehension

Ex :

std::cout << euevwilwkwjwvegz3hebeve
<< Eiegeiwisishehe << whaheiwowkwh
<< std::endl;

See what I mean by common sense ?

JonathanDUFOUR commented 7 months ago

Sure, sorry for not having provided some examples, I meant the following style would be great to use:

void foo()
{
    char const c = 'Q';
    int n = 125;
    std::vector<short> v = { 0, 1, 2, 3, 4 };

    // instructions
    if (/* condition */)
    {
        unsigned long bar = -42;

        // instructions that need c, n, v, and bar
    }
    else
    {
        char muf = '*';

        // instructions that need c, n, v, and muf
    }
...
}

Concerning the doxygen comments, I agree that some functions that are very short and self-explicit may not require a comment, but I suggest that each exception to the rule of having a doxygen comment above every function will require the approval of the other members.

Is that okay with you too?

Finally, I would notice that some extensions exist to auto-format the code, according to a set of rules we would have defined. Prettier is one of these, and I suggest we use it during the development of the project.

JonathanDUFOUR commented 5 months ago

Also about the variable declarations, I am a bit more comfortable with having an empty line after each block of variable declaration, like this:

int my_function(void)
{
    int a = another_function();

    if (/* condition */)
    {
        unsigned char b;
        float c = 3.14f;

        if (/* condition */)
        {
            /* instructions */
        }

        short d = 42;

        /* instructions */

        int e;
        std::vector<char> f;

        /* instructions */
    }
}

instead of this:

int my_function(void)
{
    int a = another_function();
    if (/* condition */)
    {
        unsigned char b;
        float c = 3.14;
        if (/* condition */)
        {
            /* instructions */
        }
        short d = 42;
        /* instructions */
        int e;
        std::vector<char> f;
        /* instructions */
    }
}
JonathanDUFOUR commented 5 months ago

Another point to decide: how many characters a line should be at most long? Usually, I write lines that are never longer than 100 in Rust and C when I have no other constraints. But sometimes, I extend this limit to 120 characters in C++. Do you have any preference?

JonathanDUFOUR commented 5 months ago

Additional suggestion: whenever we have a static function in a file, it would be a great idea to always add the inline keyword to it, and to prefix its name with a double underscore, to easily know that a function is static. Example:

inline static void __my_static_function(void)
{
  /* Instructions */
}
JonathanDUFOUR commented 4 months ago

After investigating how the arguments are passed to functions in x86-64 architecture, I suggest allowing 6 parameters per function as a maximum instead of 5.

JonathanDUFOUR commented 3 months ago

I would add another rule to our norm concerning how we access class fields and call class methods from the class itself. Indeed, I suggest that we always explicitly use the prefix this-> when doing so.