cppbestpractices/04-Considering_Safety.md

Also, using const & prevents the compiler from copying data unnecessarily.

If you internally copy object to internal var, the better variant will be passing params by value.

First example marked as "Bad Idea":

// Bad Idea
//...
MyClass(std::string t_value)
    : m_value(t_value)
  {
  }
// ...
std::string m_value;

When you do m_value(t_value) in constructor initialization list, it is anyway will be a copy, it doesn't matter you pass it by const ref or by value.

But passing by value in such situations (when you copy contents to internal variable) is much more better: it is thread-safe, no dangling refs, you can pass rvalues, you can construct in-place, etc.

There will not be two copies, see copy elision.

So, in given example it is not a Bad Idea.

In the "Use The Tools Available" section, it says that bazel is "MacOS and Linux only." According to bazel's documentation, Windows is supported: https://docs.bazel.build/versions/master/windows.html.

It says "This article provides a background and explains techniques for implementing nearly 100% of the time." which doesn't make sense.

Maybe "This article provides a background and explains techniques for implementing The Rule of Zero in the vast majority of cases", or something like that. Hard to tell, but it should be fixed.

In the 03-Style document, I read the following sentence:

If the member variable is not expected to change after the initialization, then mark it const.

However, we know that if, for example, we mark a container as const, we prevent it from being moved, possibly hindering performance. Marking fields as const is useful to enforce invariants; however, it "interferes" with performance. If both const correctness and performance are important, one could leave the field non-const, make it private and define a field getter.

So this is mostly from a discussion perspective. I'm not experienced when it comes to C++, but I'm thinking about this discussion of limited scope.

I'm mostly concerned that we might spend too much time initializing objects. Assume I have a large object (say an image) that I want to run an operation on to get some value, and I further run several operations per picture (running different sorts of filters, etc. etc.) each of these operations needing their own copy of the image that they run their operations on. Further, all of this happens several times a second, since it comes from a video stream.

I would have some function treat_frame() working with each frame, and have that function call functions for each operation. Wouldn't limiting the scope of the working objects (having them local to each function) cause constructors and destructors for these quite large objects several times every second? From my perspective it seems wiser to allocate them once outside the scope of the loop, and then have them use the same memory over and over.

Not sure how far this goes though. I did something like the above in a university project, and in profiling it turned out a lot of time was spent in constructors and allocators. Not sure how it would be with something like the example code. Like, would the constructor get invoked every iteration in the below code?

// Good Idea
for (int i = 0; i < 15; ++i)
{
  MyObject obj(i);
  // do something with obj
}

Very glad to hear that pre-increment should be preferred. However, your reasons are easily debunked in modern times where compilers will optimize both versions to the same machine level instructions. However, this argument breaks as soon as more complicated types are used: iterators. Your example uses plain old ints, but it would be clearer if you elaborated a little by using (user-defined) classes that overload ++.

I don't understand this sentence:

By using t_ for parameters and m_ for module data, we can have consistency with both public members of structs and private members of classes.

If we want to execute same procedure on several members of same type, we often use parameters like A::*var to generalize a method. Surely this could also be done by template, but we prefer the A::*var way.
Would you consider this as good style, or prefare to do thinks like this another way?


Example Code

The section below was taken directly from the document. Did you mean camelCase or PascalCase? I assumed camelCase. I'll be happy to open a pull request with this correction.

Style

Consistency is the most important aspect of style. The second most important aspect is following a style that the average C++ programmer is used to reading.

C++ allows for arbitrary-length identifier names, so there's no reason to be terse when naming things. Use descriptive names, and be consistent in the style.

• CamelCase
• camelCase
• snake_case

08-Considering_Performance.md
in the section Char is a char, string is a string (third from last)
How the performance in CPU different for "\n" and '\n' ?

There's a really good point in Style.md on how not to use asserts, but should we document when to use them as well? I think the c++ usage is quite different to other languages, where asserts are used for test code but not for dev code.

The MSVC compiler flag /W44640 is not descripted in chapter MSVC

/W4 /W44640 - use these and consider the following

The links to the old gitbook page, purported to provide downloadable editions, are now dead links. Particularly https://www.gitbook.com/book/lefticus/cpp-best-practices , which just lands on the generic GitBook homepage.

As a workaround, I am using the release artifacts posted to GitHub.

Sorry, I've accidentally deleted the wiki's main file. I thought it would redirect me to fork the repo to do that... 😑
When I tried to revert the commit that destroyed the Initial-Outline.md, github said that I don't have writing permissions (but how it allowed me to delete the file in first place?).

Can anybody revert it?

Sorry again.

My argument is that avoiding all-uppercase for all non-macros avoids those hard to understand errors you get when you use a constant or enum value that was already defined in some external header.

Ever had to do #undef ERROR after including Windows.h ?

Currently the guide recommends against /Wall for MSVC because the system headers do not comply. https://blogs.msdn.microsoft.com/vcblog/2017/12/13/broken-warnings-theory/ has an experimental feature that lets you turn off warnings for headers outside of your control.

/external:anglebrackets – a switch that allows a user to treat all headers included via #include <> (as opposed to #include "") as external headers

Might be good behavior to use the above switch assuming your MSVC compiler supports it.

While "Const as Much as Possible" is generally good practice, const should not be directly used on function declaration parameters, including member functions.

A quick example:

void foo (int x);
void foo (int const x) {return x * 2;}

void bar (char const * s);
void bar (char const * const s) {puts(s);}

Notice I said "directly". Hence s isn't const in the declaration, but is in the definition (direct), but what s points at is const in both, and must match in both (indirect).

The reason for doing this is to not leak a function's implementation details. Because whether a function treats a parameter variable as const or not is an implementation detail of the function. And if a function gets modified such that its parameter is no longer treated as const, then callers of that function shouldn't have to be recompiled.

This is a difference between C and C++. In C, the function definition's parameter type(s) must exactly match that of the function declarations. (At least it used to. I haven't actually developed anything significant with modern C.)

https://github.com/lefticus/cppbestpractices/blob/master/03-Style.md#use-the-correct-integer-type-for-standard-library-features

good rules to follow, but you might like to add a remark about computing the difference between two sizes, because even when using auto you might get unexpected behaviour, ie. value underrun:

std::size_t a=3, b = 5;
auto c = a-b;

c is now some huge number, because c is still unsigned, and you don't get a warning. Also worth noting is that because std::size_t is generally defined as the largest unsigned integer type on a system, generally there is no signed type that will hold all values of a std::size_t. Thus the only safe way of computing the difference requires that you compare b<=a before you compute the difference and take appropriate action if b>a.

Regarding "Forward Declare When Possible", I think you should (almost) never forward declare for the following reasons:

1. It makes backward compatible refactorings more complicated and prone to breakage. This is the reason that the the C++ Standard Library and abseil forbid forward declarations. In C++ you can often refactor a function into a lambda or function object or into a template and keep backward compatibility with uses of the function. However, the signature will have changed.

2. It is dangerous. Forward declarations are an easy way to get hard to diagnose errors like ODR violations. In addition, if something changed in a header you were depending on, forward declarations postpone the error to link-time or to compile time.

3. It is a short term optimization. Hopefully modules in C++ will come soon and make this better. But even without modules, such techniques as pre-compiled headers are safer and work in more scenarios.

Based on these, I think better advice would be to avoid forward declarations.

Hello,

In your 03-Style.md best practice file you talk about using nullptr instead of 0 or NULL which is a very good things to do.

There is a flag in GCC that warn the user about using 0 (or NULL) instead of nullptr: -Wzero-as-null-pointer-constant. Unfortunately this flag does not exists for clang and I don't know if it does for MSVC.

I think this should be a good idea to add this flag in your warning flag list (https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang).

Best regards

A counter point to the std::bind section seems to prove there isn't a runtime overhead for the example given: https://godbolt.org/g/ZcUgRr

Summary: There's a really easy way to do this, and keep it uptodate - this ticket is to explore whether you would accept a pull-request to implement it...

Motivation

Having seen the Table of Contents links on some docs on github, I've found them to be really useful in getting any overview of the topics covered, and then also for really easy navigation.

Examples are:

• https://github.com/catchorg/Catch2/blob/master/docs/assertions.md#top
• https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/Contributing.md#top

I keep coming back to https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md to look for different types of information, and it would save a lot of time and scrolling if users could jump to the section of interest.

Alternatives considered

Searching through the existing issues, I saw #9 which said the solution was to use gitpages,

I downloaded the gitpages PDF, and it doesn't have tables of contents for sections.

And its "Read" feature gives "This site can't be reached" - ERR_CONNECTION_TIMED_OUT - https://lefticus.gitbooks.io/cpp-best-practices/content/

Given how quickly github.com loads Markdown pages, coupled with the convenience of being able to see page histories there and even possibly suggest improvements to the docs, I feel that it's preferable to add Table of Contents to each of the live pages on github.

Possible implementation

For ApprovalTests.cpp, we are using https://github.com/thlorenz/doctoc

It's easy to install:

npm install -g doctoc

Then we run it with this on Windows:

doctoc --title **Contents** .

or this on Unix:

doctoc --title '**Contents**' .

A minor enhancement is to move the generated ToC to after the level-1 heading, so that only level-2 and above headings are included in the ToC.

Consequences

The nice this is that this doesn't require all cppbestpractices contributors to have this tool.

Many edits won't change the ToC, and for those that do, doctoc could be run later by someone who has the tool. (I'd be happy to do that, but it would be better done by someone who has commit-permission on the repo)

Static Analysis
Flaw Finder -- https://dwheeler.com/flawfinder/

Other Tools
Source Trail -- https://www.sourcetrail.com/
Herb Grind -- http://herbgrind.ucsd.edu/
Approval Tests -- https://github.com/approvals/ApprovalTests.cpp

If you would rather me make a pull request to add these just let me know.

I was always in the - use only spaces - group for coding.
Until recently, when I started using clang-format. It has many options:

UseTab (UseTabStyle)
The way to use tab characters in the resulting file.
Possible values:
UT_Never (in configuration: Never) Never use tab.
UT_ForIndentation (in configuration: ForIndentation) Use tabs only for indentation.
UT_ForContinuationAndIndentation (in configuration: ForContinuationAndIndentation) Use tabs only for line continuation and indentation.
UT_Always (in configuration: Always) Use tabs whenever we need to fill whitespace that spans at least from one tab stop to the next one.

Now I use the setting: ForIndentation
Which uses Tabs for indenting the the block, and if more indentation is required for aligning, then spaces are used.
So it doesn't matter wether someone has 2, 4, or 8 spaces per tab, the code is always aligned correctly.

Currently my stance is: use Tab for indenting, use space for aligning.

To facilitate navigation of this document directly, perhaps we should add "previous" "next" and "Table of Contents" links to the top and bottom of each page?

Regarding this: https://github.com/lefticus/cppbestpractices/blob/master/04-Considering_Safety.md#do-not-define-a-variadic-function

Rather than avoiding C-style variadic functions entirely, you should advise to enable type checking for them. This is included in -Wall; you should also suggest -Wformat=2 in the list of useful warnings, as this warns about non-literal format strings etc.

And in the section which currently says not to define variadic functions, you should say, "unless they can be type-checked by your compiler." And if the function is similar to printf(), scanf(), strftime(), etc., it can be. See the "format" attribute described here: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes .

There are times when C-style variadic functions are most convenient, such as making a strftime() method on a user-defined time class. And they can be safe enough.

I know there is a closed issue about this already. But I want to add a point here against using prefixes like this:

They make the code less readable. You say on the other hand it will be easier to recognize where a variable comes from / what it's scope is (which would be better readability). But this is not the responsibility of code typing style. The responsibility of making the scope / origin of a variable clear is only on the IDE / editors side (syntax highlighting, auto-highlighting selection, outline views and the like).

So, since we nowadays have editors doing all this for you, I'd say prefixing takes a greater amount of readability than it gives, because editors already do provide the positive effects of prefixing so that only the negative aspects of prefixing stay.

It is stated:

Source control is an absolute necessity for any software development project. If you are not using one yet, start using one.

Which is true, but then there is a list of source control hosting services, which are not the same thing.

I'd like to add the ABI compliance checker (http://ispras.linuxbase.org/index.php/ABI_compliance_checker) to the tools section. It can analyze two library versions and generates a detailed compatibility report regarding API and C++ ABI changes.

I would have opened a MR, but I cannot find a section where this tools fits among the existing ones. Shall I add a new section?

was fine yesterday: https://lefticus.gitbooks.io/cpp-best-practices/content/

coitre declares itself as depreacted:

from https://github.com/sakra/cotire/:

The functionality provided by cotire has been superseded by features added to CMake 3.16. Support for pre-compiling and unity builds is now built into CMake. Thus, there will not be any further updates or support for this project.

The gitbook link in the readme links to the legacy version of gitbook, new gitbook users are unable to login through the legacy portal. Get 401 error when I go to login after clicking the gitbook link in the readme. I can still download the PDF book and read it in an online viewer, but if I wanted to preview the book through gitbook's website I would not be able to.

Why we should use "t_" prefix for function parameters? What is the practical sense? I think it's just difficult to read. The function parameters should not be prefixed.

https://github.com/lefticus/cppbestpractices/blob/master/03-Style.md#distinguish-function-parameters

Many links seem to be awkwardly worded or raw URLs. Like this one from the "Considering Threadability" section:

For more information see the following article from Herb Sutter: http://herbsutter.com/2013/05/24/gotw-6a-const-correctness-part-1-3/
Instead, how about something more readable?

For more information see [Const Correctness, Part 1](http://herbsutter.com/2013/05/24/gotw-6a-const-correctness-part-1-3/) by Herb Sutter.

In section "Avoid Raw Memory Access" using of std::unique_ptr can be added for arrays:

auto my_data = std::make_unique<MyDataClass>(128);

In the Use the tools available section it is recommended to enable the flag -Wnon-virtual-dtor, and there is a note that says
-Weffc++ warning mode can be too noisy, but if it works for your project, use it also. however, according to clang's documentation, these two flags are synonyms https://clang.llvm.org/docs/DiagnosticsReference.html#weffc

Instead of commenting out the code with /* */ you can use:

#if 0
void function_f(int param)
{
  (void)param;
}
#endif

In this case you can nest such commented out parts. Moreover, syntax highlighting still work in some IDEs.

03-Style - line 368

The text mentions "integer underflow". There is no such thing. Underflow (that is, truncation to zero) happens only for floating point types.

https://stackoverflow.com/questions/6360049/what-are-arithmetic-underflow-and-overflow-in-c

I'd like to make the observation that perhaps this is a good place for templated functions instead of runtime checks.
If so, how often would you see this in production code?

In the safety module, there is an example recommending using const to indicate an immutable getter method:
std::string get_value() const { /* etc */ }

However, I think that best practices actually recommend going slightly further by also returning a const reference. Like this:
std::string const &get_value() const { /* etc */ }

Can anyone confirm or deny this practice? Is it a best practice?

Insofar as "best practices" goes:

When I see lefticus' github repository list, I think this privately:

*face palm* one of these guys with the crazy disorganized repositories list and a bunch of mixed in random forks of other projects and random postings of small insignificant test projects so that I cannot see the list of projects that he actually created and maintains and I have to scroll through for 10 minutes figuring out what-is-what

How to alleviate this problem:

1. Create a user called lefticusprivate
2. Move all your junk and test projects and random forks there
3. Leave your serious and significant projects on lefticus.

https://www.youtube.com/c/JasonTurner-lefticus leeds to 404

Maybe replace it with https://www.youtube.com/user/lefticus1

Best regards and i enjoy learning from you

• please remove -

Source Control - there are almost too many to mention. CVS/Subversion/Bitkeeper etc.
Internal versus external.

Build Tool - automake/autoconfig, make in general (pros/cons)?

Continuous Integration - I don't have any experience with this. Typically I have worked in groups where one person is in charge of integrating and running tests. Or having a check in system where every night tests are run to test the system as a whole.

GCC / Clang -- I like -Wunused
cppcheck - Therefor spelling...

-- I love lcov and gcov

Unit Tests -- distinction with "Tests".

For me, unit tests are great for an encapsulated object where it is really easy to test inputs and outputs - especially when the object is initially created.

Tests are needed to test the functionality of all of the code.

And not exactly a "test" but is related -- the concept of objects and methods checking inputs to verify that the input is correct or in range before continuing.

It is stated here that

Operations on doubles are typically faster than floats.

I can not reproduce this. For my tests, doing math on floats and doubles is equally fast if SSE is not considered. I would even have expected that floats would be faster due to memory bandwidth but I am having trouble testing that since gcc keeps autovectorizing my code.

Anyway, can anyone confirm or deny the statement that doubles are faster than floats anywhere?

Section 03 Style - Consider the Rule of Zero

The Rule of Zero states that you do not provide any of the functions that the compiler can provide (copy constructor, assignment operator, move constructor, destructor, move constructor) unless the class you are constructing does some novel form of ownership.

The current wording reads "/Wall - Also warns on files included from the standard library, so it's not very useful and creates too many extra warnings." – this reason does not seem to hold, because /external (i.e. /external:anglebrackets) exists and is effective in preventing /Wall warnings in the standard headers too.

A better reason to avoid /Wall would be a large number of informational diagnostics (i.e. https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4711?view=msvc-170) - but then again, these can be disabled individually.