Changeset View
Changeset View
Standalone View
Standalone View
doc/developer-notes.md
Show All 9 Lines | |||||
- Basic rules specified in [src/.clang-format](/src/.clang-format). | - Basic rules specified in [src/.clang-format](/src/.clang-format). | ||||
- Braces on new lines for namespaces, classes, functions, methods. | - Braces on new lines for namespaces, classes, functions, methods. | ||||
- Braces on the same line for everything else. | - Braces on the same line for everything else. | ||||
- 4 space indentation (no tabs) for every block except namespaces. | - 4 space indentation (no tabs) for every block except namespaces. | ||||
- No indentation for `public`/`protected`/`private` or for `namespace`. | - No indentation for `public`/`protected`/`private` or for `namespace`. | ||||
- No extra spaces inside parenthesis; don't do ( this ) | - No extra spaces inside parenthesis; don't do ( this ) | ||||
- No space after function names; one space after `if`, `for` and `while`. | - No space after function names; one space after `if`, `for` and `while`. | ||||
- If an `if` only has a single-statement then-clause, it can appear | - If an `if` only has a single-statement then-clause, it can appear | ||||
jimpo: Remove the allowance for single-line if statements. | |||||
on the same line as the if, without braces. In every other case, | on the same line as the if, without braces. In every other case, | ||||
braces are required, and the then and else clauses must appear | braces are required, and the then and else clauses must appear | ||||
correctly indented on a new line. | correctly indented on a new line. | ||||
- `++i` is preferred over `i++`. | - `++i` is preferred over `i++`. | ||||
Block style example: | Code style example: | ||||
```c++ | ```c++ | ||||
namespace foo | // namespaces should be lower_snake_case | ||||
namespace foo_bar_bob | |||||
{ | { | ||||
jasonbcoxUnsubmitted Not Done Inline ActionsCurly braces should be on the same line as the definition. Same goes for lines below this one. jasonbcox: Curly braces should be on the same line as the definition. Same goes for lines below this one. | |||||
/** | |||||
* Class is used for doing classy things. All classes should | |||||
* have a doxygen comment describing their PURPOSE. That is to say, | |||||
* why they exist. Functional details can be determined from the code. | |||||
* @see PerformTask() | |||||
*/ | |||||
class Class | class Class | ||||
{ | { | ||||
bool Function(const std::string& s, int n) | private: | ||||
//! memberVariable's name should be lowerCamelCase, and be a noun. | |||||
jimpoUnsubmitted Not Done Inline ActionsPresumably lowerCamelCase for local variables too? Can we document that somewhere? jimpo: Presumably lowerCamelCase for local variables too? Can we document that somewhere? | |||||
int memberVariable; | |||||
public: | |||||
/** | |||||
* The documentation before a function should follow Doxygen spec. | |||||
jimpoUnsubmitted Not Done Inline ActionsCan you make it explicit that this applies to both functions and class methods? jimpo: Can you make it explicit that this applies to both functions and class methods? | |||||
* The name of the function should start with an english verb which indicates | |||||
* what it is doing. It should be CamelCase. | |||||
* @param[in] s A description | |||||
* @param[in] n Another argument description | |||||
* @pre Precondition for function... | |||||
*/ | |||||
bool PerformTask(const std::string& s, int n) | |||||
{ | { | ||||
// Comment summarising what this section of code does | // Comment summarising what this section of code does | ||||
jasonbcoxUnsubmitted Not Done Inline ActionsRather than what this code does, perhaps "Comment summarizing the intended purpose of this section of code". Unless the code is particularly difficult to parse, describing what it does is a bit redundant. The intended purpose is often very similar to what it does, but we all know that isn't always the case. :P jasonbcox: Rather than what this code does, perhaps "Comment summarizing the intended purpose of this… | |||||
for (int i = 0; i < n; ++i) { | for (int i = 0; i < n; ++i) { | ||||
// When something fails, return early | // When something fails, return early | ||||
if (!Something()) return false; | if (!IsSomething()) return false; | ||||
jimpoUnsubmitted Not Done Inline ActionsBraces. jimpo: Braces. | |||||
... | ... | ||||
if (SomethingElse()) { | if (IsSomethingElse()) { | ||||
DoMore(); | DoMore(); | ||||
} else { | } else { | ||||
DoLess(); | DoLess(); | ||||
} | } | ||||
} | } | ||||
// Success return is usually at the end | // Success return is usually at the end | ||||
return true; | return true; | ||||
} | } | ||||
} | } | ||||
} | } | ||||
``` | ``` | ||||
The naming convention roughly follows [Microsoft Naming Conventions](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/general-naming-conventions) | |||||
C++ Coding Standards should strive to follow the [LLVM Coding Stanadrds](https://llvm.org/docs/CodingStandards.html) | |||||
Doxygen comments | Doxygen comments | ||||
----------------- | ----------------- | ||||
To facilitate the generation of documentation, use doxygen-compatible comment blocks for functions, methods and fields. | To facilitate the generation of documentation, use doxygen-compatible comment blocks for functions, methods and fields. | ||||
For example, to describe a function use: | For example, to describe a function use: | ||||
```c++ | ```c++ | ||||
/** | /** | ||||
▲ Show 20 Lines • Show All 164 Lines • ▼ Show 20 Lines | |||||
and commit them. | and commit them. | ||||
Development guidelines | Development guidelines | ||||
============================ | ============================ | ||||
A few non-style-related recommendations for developers, as well as points to | A few non-style-related recommendations for developers, as well as points to | ||||
pay attention to for reviewers of Bitcoin Core code. | pay attention to for reviewers of Bitcoin Core code. | ||||
General Bitcoin Core | |||||
---------------------- | |||||
- New features should be exposed on RPC first, then can be made available in the GUI | |||||
- *Rationale*: RPC allows for better automatic testing. The test suite for | |||||
the GUI is very limited | |||||
- Make sure pull requests pass Travis CI before merging | |||||
- *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing | |||||
on the master branch. Otherwise all new pull requests will start failing the tests, resulting in | |||||
confusion and mayhem | |||||
- *Explanation*: If the test suite is to be updated for a change, this has to | |||||
be done first | |||||
Wallet | Wallet | ||||
------- | ------- | ||||
- Make sure that no crashes happen with run-time option `-disablewallet`. | - Make sure that no crashes happen with run-time option `-disablewallet`. | ||||
- *Rationale*: In RPC code that conditionally uses the wallet (such as | - *Rationale*: In RPC code that conditionally uses the wallet (such as | ||||
`validateaddress`) it is easy to forget that global pointer `pwalletMain` | `validateaddress`) it is easy to forget that global pointer `pwalletMain` | ||||
can be NULL. See `test/functional/disablewallet.py` for functional tests | can be NULL. See `test/functional/disablewallet.py` for functional tests | ||||
▲ Show 20 Lines • Show All 60 Lines • ▼ Show 20 Lines | |||||
- Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior | - Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior | ||||
- *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those | - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those | ||||
that are not language lawyers | that are not language lawyers | ||||
Strings and formatting | Strings and formatting | ||||
------------------------ | ------------------------ | ||||
- Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not. | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsShould we leave this in until T214 is completed? jasonbcox: Should we leave this in until T214 is completed? | |||||
- *Rationale*: Confusion of these can result in runtime exceptions due to | |||||
formatting mismatch, and it is easy to get wrong because of subtly similar naming | |||||
- Use `std::string`, avoid C string manipulation functions | - Use `std::string`, avoid C string manipulation functions | ||||
- *Rationale*: C++ string handling is marginally safer, less scope for | - *Rationale*: C++ string handling is marginally safer, less scope for | ||||
buffer overflows and surprises with `\0` characters. Also some C string manipulations | buffer overflows and surprises with `\0` characters. Also some C string manipulations | ||||
tend to act differently depending on platform, or even the user locale | tend to act differently depending on platform, or even the user locale | ||||
- Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing | - Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing | ||||
- *Rationale*: These functions do overflow checking, and avoid pesky locale issues | - *Rationale*: These functions do overflow checking, and avoid pesky locale issues | ||||
- For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers | |||||
- *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion | |||||
Variable names | Variable names | ||||
-------------- | -------------- | ||||
The shadowing warning (`-Wshadow`) is enabled by default. It prevents issues rising | The shadowing warning (`-Wshadow`) is enabled by default. It prevents issues rising | ||||
from using a different variable with the same name. | from using a different variable with the same name. | ||||
Please name variables so that their names do not shadow variables defined in the source code. | Please name variables so that their names do not shadow variables defined in the source code. | ||||
E.g. in member initializers, prepend `_` to the argument name shadowing the | |||||
member name: | |||||
```c++ | |||||
class AddressBookPage | |||||
{ | |||||
Mode mode; | |||||
} | |||||
AddressBookPage::AddressBookPage(Mode _mode) : | |||||
mode(_mode) | |||||
... | |||||
``` | |||||
When using nested cycles, do not name the inner cycle variable the same as in | |||||
upper cycle etc. | |||||
Threads and synchronization | Threads and synchronization | ||||
---------------------------- | ---------------------------- | ||||
- Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential | - Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential | ||||
deadlocks are introduced. As of 0.12, this is defined by default when | deadlocks are introduced. As of 0.12, this is defined by default when | ||||
configuring with `--enable-debug` | configuring with `--enable-debug` | ||||
- When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of | - When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of | ||||
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines |
Remove the allowance for single-line if statements.