Difference between revisions of "Coding Style"
(Clean this up a bit until (if) the new Coding Style gets published.) |
Colin Finck (talk | contribs) (FINALLY, here it is! Our new coding style document based on https://docs.google.com/document/d/1qIWw6FN2mw-Fws2lTFDUZ0RV1nvL44VccN4C2oe5kIU/edit?usp=sharing) |
||
Line 1: | Line 1: | ||
− | The ReactOS | + | This article describes general coding style guidelines, which should be used for new ReactOS code. These guidelines apply exclusively to C and C++ source files. The Members of ReactOS agreed on this document in the October 2013 meeting. |
− | + | As much existing ReactOS code as possible should be converted to this style unless there are reasons against doing this (like if the code is going to be rewritten from scratch in the near future). See [[#Notes on reformatting existing code|Notes on reformatting existing code]] for more details. | |
− | |||
− | + | Code synchronized with other sources (like Wine) must not be rewritten. | |
− | |||
− | == | + | == File Structure == |
− | + | <ol> | |
+ | <li>Every ReactOS source code file should include a file header like this: | ||
+ | <syntaxhighlight lang="c"> | ||
+ | /* | ||
+ | * PROJECT: ReactOS Kernel | ||
+ | * LICENSE: GNU GPLv2 only as published by the Free Software Foundation | ||
+ | * PURPOSE: Does cool things like ... | ||
+ | * PROGRAMMERS: Arno Nymous (abc@mailaddress.com) | ||
+ | * Mike Blablabla (mike@blabla.com) | ||
+ | */ | ||
+ | </syntaxhighlight> | ||
+ | |||
+ | We need to keep some consistency and ensure that licenses can be identified uniquely. Keep in mind that some licenses may be ReactOS-specific. Therefore, please use a precise name for the license and give information where to obtain it. | ||
− | === | + | '''Examples:''' |
− | + | * <tt>GNU GPLv2 only as published by the Free Software Foundation</tt> | |
+ | * <tt>GNU GPLv2 or any later version as published by the Free Software Foundation</tt> | ||
+ | * <tt>BSD - See COPYING.ARM in the top-level directory</tt> | ||
+ | |||
+ | |||
+ | You may add yourself to the <tt>PROGRAMMERS</tt> section of a file if you did a major contribution to it and could take responsibility for the whole file or a part of it. | ||
+ | </li> | ||
+ | |||
+ | <li>Use a header comparable to the following one for functions: | ||
+ | <syntaxhighlight lang="c"> | ||
+ | /** | ||
+ | * @name SomeAPI | ||
+ | * @implemented | ||
+ | * | ||
+ | * Do nothing for 500ms. | ||
+ | * | ||
+ | * @param SomeParameter | ||
+ | * Description of the parameter. | ||
+ | * | ||
+ | * @param YetAnotherParameter | ||
+ | * Bleh, bleh :) | ||
+ | * | ||
+ | * @return | ||
+ | * STATUS_SUCCESS in case of success, STATUS_UNSUCCESSFUL | ||
+ | * otherwise. | ||
+ | * | ||
+ | * @remarks Must be called at IRQL == DISPATCH_LEVEL | ||
+ | * | ||
+ | */ | ||
+ | </syntaxhighlight> | ||
+ | </li> | ||
+ | </ol> | ||
+ | |||
+ | == Indentation == | ||
+ | <ol> | ||
+ | <li>Indent with 4 spaces, don’t use tabs!</li> | ||
+ | <li><p>Indent both a case label and the case statement of a switch statement.</p> | ||
+ | |||
+ | '''Right:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | switch (Condition) | ||
+ | { | ||
+ | case 1: | ||
+ | DoSomething(); | ||
+ | break; | ||
+ | } | ||
+ | </syntaxhighlight> | ||
− | + | '''Wrong:''' | |
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | switch (Condition) | |
− | + | { | |
+ | case 1: | ||
+ | DoSomething(); | ||
+ | break; | ||
} | } | ||
+ | </syntaxhighlight> | ||
+ | </li> | ||
+ | </ol> | ||
+ | |||
+ | == Spacing == | ||
+ | <ol> | ||
+ | <li>Do not use spaces around unary operators.<br /> | ||
+ | '''Right:''' <code>i++;</code><br /> | ||
+ | '''Wrong:''' <code>i ++;</code> | ||
+ | </li> | ||
+ | |||
+ | <li>Place spaces around binary and ternary operators.<br /> | ||
+ | '''Right:''' <code>a = b + c;</code><br /> | ||
+ | '''Wrong:''' <code>a=b+c;</code> | ||
+ | </li> | ||
+ | |||
+ | <li><p>Do not place spaces before comma and semicolon.</p> | ||
+ | |||
+ | '''Right:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | for (int i = 0; i < 5; i++) | ||
+ | DoSomething(); | ||
+ | |||
+ | func1(a, b); | ||
+ | </syntaxhighlight> | ||
+ | |||
+ | '''Wrong:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | for (int i = 0 ; i < 5 ; i++) | ||
+ | DoSomething(); | ||
+ | |||
+ | func1(a , b) ; | ||
+ | </syntaxhighlight> | ||
+ | </li> | ||
+ | |||
+ | <li><p>Place spaces between control statements and their parentheses.</p> | ||
+ | |||
+ | '''Right:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | if (Condition) | ||
+ | DoSomething(); | ||
+ | </syntaxhighlight> | ||
+ | |||
+ | '''Wrong:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | if(Condition) | ||
+ | DoSomething(); | ||
+ | </syntaxhighlight> | ||
+ | </li> | ||
+ | |||
+ | <li><p>Do not place spaces between a function and its parentheses, or between a parenthesis and its content.</p> | ||
+ | |||
+ | '''Right:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | func(a, b); | ||
+ | </syntaxhighlight> | ||
+ | |||
+ | '''Wrong:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | func (a, b); | ||
+ | func( a, b ); | ||
+ | </syntaxhighlight> | ||
+ | </li> | ||
+ | </ol> | ||
+ | |||
+ | == Line breaking == | ||
+ | <ol> | ||
+ | <li><p>Each statement should get its own line.</p> | ||
− | if ( | + | '''Right:''' |
− | + | <syntaxhighlight lang="c"> | |
− | + | x++; | |
+ | y++; | ||
+ | |||
+ | if (Condition) | ||
+ | DoSomething(); | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | ||
+ | '''Wrong:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | if ( | + | x++; y++; |
+ | |||
+ | if (Condition) DoSomething(); | ||
+ | </syntaxhighlight> | ||
+ | </li> | ||
+ | </ol> | ||
+ | |||
+ | == Braces == | ||
+ | <ol> | ||
+ | <li>Always put braces (''{'' and ''}'') on their own lines.</li> | ||
+ | <li><p>One-line control clauses may use braces, but this is not a requirement. An exception are one-line control clauses including additional comments.</p> | ||
+ | |||
+ | '''Right:''' | ||
+ | <syntaxhighlight lang="c"> | ||
+ | if (Condition) | ||
+ | DoSomething(); | ||
+ | |||
+ | if (Condition) | ||
{ | { | ||
− | + | DoSomething(); | |
} | } | ||
+ | if (Condition) | ||
+ | { | ||
+ | // This is a comment | ||
+ | DoSomething(); | ||
+ | } | ||
+ | |||
+ | if (Condition) | ||
+ | DoSomething(); | ||
+ | else | ||
+ | DoSomethingElse(); | ||
+ | |||
+ | if (Condition) | ||
+ | { | ||
+ | DoSomething(); | ||
+ | } | ||
+ | else | ||
+ | { | ||
+ | DoSomethingElse(); | ||
+ | YetAnother(); | ||
+ | } | ||
+ | </syntaxhighlight> | ||
− | if ( | + | '''Wrong:''' |
− | + | <syntaxhighlight lang="c"> | |
+ | if (Condition) { | ||
+ | DoSomething(); | ||
+ | } | ||
+ | if (Condition) | ||
+ | // This is a comment | ||
+ | DoSomething(); | ||
− | if ( | + | if (Condition) |
+ | DoSomething(); | ||
+ | else | ||
{ | { | ||
− | + | DoSomethingElse(); | |
− | + | YetAnother(); | |
} | } | ||
</syntaxhighlight> | </syntaxhighlight> | ||
+ | </li> | ||
+ | </ol> | ||
− | === | + | == Control structures == |
− | + | <ol> | |
+ | <li>Don’t use inverse logic in control clauses.<br /> | ||
+ | '''Right:''' <code>if (i == 1)</code><br /> | ||
+ | '''Wrong:''' <code>if (1 == i)</code> | ||
+ | </li> | ||
− | + | <li><p>Avoid too many levels of cascaded control structures. Prefer a “linear style” over a “tree style”. Use <code>goto</code> when it helps to make the code cleaner (e.g. for cleanup pathes).</p> | |
− | |||
− | + | '''Right:''' | |
− | |||
− | |||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | if (!func1()) | |
− | + | return; | |
− | + | ||
− | + | i = func2(); | |
− | + | if (i == 0) | |
− | + | return; | |
− | + | ||
− | + | j = func3(); | |
− | + | if (j == 1) | |
− | + | return; | |
+ | … | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | ||
+ | '''Wrong:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | if (func1()) | |
+ | { | ||
+ | i = func2(); | ||
+ | if (func2()) | ||
+ | { | ||
+ | j = func3(); | ||
+ | if (func3()) | ||
+ | { | ||
+ | … | ||
+ | } | ||
+ | } | ||
+ | } | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | </li> | |
+ | </ol> | ||
+ | |||
+ | == Naming == | ||
+ | <ol> | ||
+ | <li><p>Capitalize names of variables and functions.<br /> | ||
+ | [[Hungarian Notation]] may be used when developing for Win32, but it is not required. If you don’t use it, the first letter of a name must be a capital too (no camelCase). Do not use underscores as separators either.</p> | ||
+ | |||
+ | '''Right:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | PLIST_ENTRY FirstEntry; | |
+ | VOID NTAPI IopDeleteIoCompletion(PVOID ObjectBody); | ||
+ | PWSTR pwszTest; | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | ||
+ | '''Wrong:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | PLIST_ENTRY first_entry; | |
+ | VOID NTAPI iop_delete_io_completion(PVOID objectBody); | ||
+ | PWSTR pwsztest; | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | </li> | |
+ | |||
+ | <li>Avoid abbreviating function and variable names, use descriptive verbs where possible.</li> | ||
+ | <li><p>Precede boolean values with meaningful verbs like "is" and "did" if possible and if it fits the usage.</p> | ||
+ | |||
+ | '''Right:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | BOOLEAN IsValid; | |
+ | BOOLEAN DidSendData; | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | ||
+ | '''Wrong:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | BOOLEAN Valid; | |
+ | BOOLEAN SentData; | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | </li> | |
+ | </ol> | ||
+ | |||
+ | == Commenting == | ||
+ | <ol> | ||
+ | <li><p>Avoid line-wasting comments, which could fit into a single line.</p> | ||
+ | |||
+ | '''Right:''' | ||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | + | // This is a one-line comment | |
+ | |||
+ | /* This is a C-style comment */ | ||
+ | |||
+ | // | ||
+ | // This is a comment over multiple lines. | ||
+ | // We don’t define any strict rules for it. | ||
+ | // | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | '''Wrong:''' | |
− | |||
<syntaxhighlight lang="c"> | <syntaxhighlight lang="c"> | ||
− | / | + | // |
− | + | // This comment wastes two lines | |
− | + | // | |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
</syntaxhighlight> | </syntaxhighlight> | ||
+ | </li> | ||
+ | </ol> | ||
− | == | + | == Null, false and 0 == |
− | + | <ol> | |
− | + | <li>The null pointer should be written as <code>NULL</code>.<br /> | |
− | + | In the rare case that your environment recommends a different null pointer (e.g. C++11 <code>nullptr</code>), you may use this one of course. Just don’t use the value <code>0</code>.</li> | |
− | + | <li>Win32/NT Boolean values should be written as <code>TRUE</code> and <code>FALSE</code>.<br /> | |
− | + | In the rare case that you use C/C++ <code>bool</code> variables, you should write them as <code>true</code> and <code>false</code>.</li> | |
− | + | </ol> | |
− | |||
− | |||
− | |||
− | |||
− | == | + | == Notes on reformatting existing code == |
− | * | + | * Never totally reformat a file and put a code change into it. Do this in separate commits. |
− | * [ | + | * If a commit only consists of formatting changes, say this clearly in the commit message by preceding it with ''[FORMATTING]''. |
+ | == Using an automatic code style tool == | ||
+ | TO BE ADDED BY [[User:Zefklop]] | ||
− | + | == Points deliberately left out == | |
− | [[ | + | Additional ideas were suggested during the discussion of this document, but a consensus couldn't be reached on them. Therefore we refrain from enforcing any rules on these points: |
+ | * TO BE ADDED BY [[User:Hbelusca]] |
Revision as of 14:14, 1 November 2013
This article describes general coding style guidelines, which should be used for new ReactOS code. These guidelines apply exclusively to C and C++ source files. The Members of ReactOS agreed on this document in the October 2013 meeting.
As much existing ReactOS code as possible should be converted to this style unless there are reasons against doing this (like if the code is going to be rewritten from scratch in the near future). See Notes on reformatting existing code for more details.
Code synchronized with other sources (like Wine) must not be rewritten.
Contents
File Structure
- Every ReactOS source code file should include a file header like this:
/* * PROJECT: ReactOS Kernel * LICENSE: GNU GPLv2 only as published by the Free Software Foundation * PURPOSE: Does cool things like ... * PROGRAMMERS: Arno Nymous (abc@mailaddress.com) * Mike Blablabla (mike@blabla.com) */
We need to keep some consistency and ensure that licenses can be identified uniquely. Keep in mind that some licenses may be ReactOS-specific. Therefore, please use a precise name for the license and give information where to obtain it.
Examples:
- GNU GPLv2 only as published by the Free Software Foundation
- GNU GPLv2 or any later version as published by the Free Software Foundation
- BSD - See COPYING.ARM in the top-level directory
You may add yourself to the PROGRAMMERS section of a file if you did a major contribution to it and could take responsibility for the whole file or a part of it. - Use a header comparable to the following one for functions:
/** * @name SomeAPI * @implemented * * Do nothing for 500ms. * * @param SomeParameter * Description of the parameter. * * @param YetAnotherParameter * Bleh, bleh :) * * @return * STATUS_SUCCESS in case of success, STATUS_UNSUCCESSFUL * otherwise. * * @remarks Must be called at IRQL == DISPATCH_LEVEL * */
Indentation
- Indent with 4 spaces, don’t use tabs!
Indent both a case label and the case statement of a switch statement.
Right:
switch (Condition) { case 1: DoSomething(); break; }
Wrong:
switch (Condition) { case 1: DoSomething(); break; }
Spacing
- Do not use spaces around unary operators.
Right:i++;
Wrong:i ++;
- Place spaces around binary and ternary operators.
Right:a = b + c;
Wrong:a=b+c;
Do not place spaces before comma and semicolon.
Right:
for (int i = 0; i < 5; i++) DoSomething(); func1(a, b);
Wrong:
for (int i = 0 ; i < 5 ; i++) DoSomething(); func1(a , b) ;
Place spaces between control statements and their parentheses.
Right:
if (Condition) DoSomething();
Wrong:
if(Condition) DoSomething();
Do not place spaces between a function and its parentheses, or between a parenthesis and its content.
Right:
func(a, b);
Wrong:
func (a, b); func( a, b );
Line breaking
Each statement should get its own line.
Right:
x++; y++; if (Condition) DoSomething();
Wrong:
x++; y++; if (Condition) DoSomething();
Braces
- Always put braces ({ and }) on their own lines.
One-line control clauses may use braces, but this is not a requirement. An exception are one-line control clauses including additional comments.
Right:
if (Condition) DoSomething(); if (Condition) { DoSomething(); } if (Condition) { // This is a comment DoSomething(); } if (Condition) DoSomething(); else DoSomethingElse(); if (Condition) { DoSomething(); } else { DoSomethingElse(); YetAnother(); }
Wrong:
if (Condition) { DoSomething(); } if (Condition) // This is a comment DoSomething(); if (Condition) DoSomething(); else { DoSomethingElse(); YetAnother(); }
Control structures
- Don’t use inverse logic in control clauses.
Right:if (i == 1)
Wrong:if (1 == i)
Avoid too many levels of cascaded control structures. Prefer a “linear style” over a “tree style”. Use
goto
when it helps to make the code cleaner (e.g. for cleanup pathes).Right:
if (!func1()) return; i = func2(); if (i == 0) return; j = func3(); if (j == 1) return; …
Wrong:
if (func1()) { i = func2(); if (func2()) { j = func3(); if (func3()) { … } } }
Naming
Capitalize names of variables and functions.
Hungarian Notation may be used when developing for Win32, but it is not required. If you don’t use it, the first letter of a name must be a capital too (no camelCase). Do not use underscores as separators either.Right:
PLIST_ENTRY FirstEntry; VOID NTAPI IopDeleteIoCompletion(PVOID ObjectBody); PWSTR pwszTest;
Wrong:
PLIST_ENTRY first_entry; VOID NTAPI iop_delete_io_completion(PVOID objectBody); PWSTR pwsztest;
- Avoid abbreviating function and variable names, use descriptive verbs where possible.
Precede boolean values with meaningful verbs like "is" and "did" if possible and if it fits the usage.
Right:
BOOLEAN IsValid; BOOLEAN DidSendData;
Wrong:
BOOLEAN Valid; BOOLEAN SentData;
Commenting
Avoid line-wasting comments, which could fit into a single line.
Right:
// This is a one-line comment /* This is a C-style comment */ // // This is a comment over multiple lines. // We don’t define any strict rules for it. //
Wrong:
// // This comment wastes two lines //
Null, false and 0
- The null pointer should be written as
NULL
.
In the rare case that your environment recommends a different null pointer (e.g. C++11nullptr
), you may use this one of course. Just don’t use the value0
. - Win32/NT Boolean values should be written as
TRUE
andFALSE
.
In the rare case that you use C/C++bool
variables, you should write them astrue
andfalse
.
Notes on reformatting existing code
- Never totally reformat a file and put a code change into it. Do this in separate commits.
- If a commit only consists of formatting changes, say this clearly in the commit message by preceding it with [FORMATTING].
Using an automatic code style tool
TO BE ADDED BY User:Zefklop
Points deliberately left out
Additional ideas were suggested during the discussion of this document, but a consensus couldn't be reached on them. Therefore we refrain from enforcing any rules on these points:
- TO BE ADDED BY User:Hbelusca