Fall-through in switch cases is often considered risky.
Hence consider adding an unconditional break
for each switch clause.
Default case provides switch statements with fallback, and in general is a good to have. Hence consider adding default case to switch.
*
, "
, '
, \
and /*
found in header names CXX-W1207Using special-meaning characters in header names can produce errors in parsing.
Consider cleaning up header names.
for
loop modified in body CXX-W1241Modifying the control variable of a for
loop in its body can make the code harder to read.
Consider using a while loop, or move the modification into the for
loop's update expression.
Global variables are accessible anywhere in the scope of the program even in the threads outside of the main thread, as long as it shares the main process stack.
A mutable global variable have high dependency and the dependent code on such variables have no way to learn about the changes to the variable. Hence using a non-const global variables is considered quite risky, as they can lead to data races or contention issues.
Consider avoiding the use of global variable by adding a qualifier const
. If
the variable must be mutable then consider passing the variable as argument to
the functions which are designed to mutate the variable.
In the case of atomic or synchronization primitives being used to handle data races, there is still concern about misuse, which can be reduced by making it non-global. That is a private class member that can only be used by its function.
When a function returns a pointer to dynamically allocated memory, it is the responsibility of the caller to free that memory after it is no longer needed. But if the caller deferences such a pointer on the left-hand side of an assignment expression, the caller loses the oppurtunity to free the memory.
for
loop CXX-W1240The following is the syntax of a for
loop.
attr (optional) for ( declaration-or-expression (optional) ; condition (optional) ; iteration-expression (optional) ) statement
Though the last part of the loop i.e. "iteration-expression" is optional, missing it out or moving it to the body of the for loop defeats the purpose of using a for
loop.
If such is the requirement, consider using a while
.
Switch statements should have more than one clause for them to be any more useful over an if-else
construct. Consider rewriting the switch
as if-else
, thus making the code more readable.
Const or reference data members are only allowed to be initialized by the constructor of the class, and never after. And unlike in other languages, they are still instance dependent types. Having such members is rarely useful, and makes the class only copy-constructible but not copy-assignable.
A side effect refers to a modification of the state of a program or system that is outside the intended computation. Having a side-effect in an array index have multiple drawbacks. Array index with side effect(s) can be difficult to read and understand, especially for other programmers who may need to maintain or modify the code in the future. It can also make debugging more difficult.
{}
CXX-W1243Loop body not being enclosed in {}
can make it confusing and hard to read,
hence consider using {}
for loop body.
sizeof
with an expression as operand CXX-W1188The operator sizeof
never evaluates the expression provided as an argument unless it's a variable length array. It only determines the type of the operand. (See reference).
Raw string literals are used in C++ to avoid using escape characters. They can be used to express all types of string literals. A raw string literal enables you to express a string without having to perform extra construction or conversion steps. For example, if you want to express a string that contains a backslash, you can use a raw string literal to avoid having to escape the backslash.
shrink_to_fit()
on std::vector
CXX-W2027shrink_to_fit()
and the copy-swap trick are two ways to reduce the capacity of a std::vector
container in C++.
The use of deprecated C++ headers such as signal.h
and assert.h
is considered an antipattern, as they are not part of the C++ standard library anymore and have been replaced by their C++ equivalents.
ios_base
aliases CXX-W2031The use of deprecated ios_base
aliases such as ios_base::open_mode
and ios_base::seek_dir
is considered an antipattern because they have been replaced by their non-deprecated equivalents.
std::random_shuffle
type CXX-W2032std::random_shuffle
is deprecated because it uses an unspecified RNG (Random Number Generator) which can lead to unpredictable behavior. The iterator-only version of std::random_shuffle
usually depends on std::rand
, which is now also discussed for deprecation.
The use of constructor on return is considered redundant as the type has already been provided by the return type, and writing the constructor again is of no meaningful value to the program.
Using numeric types as boolean is considered antipattern, because it can lead to unexpected behavior. For example, if you use an integer type as a boolean type in C++, then any non-zero value will be
Found repeated branches in if/else statements, consecutive repeated branches in switch statements, and identical true and false branches in conditional operators.
In the bad practice example, the then
and else
branches are identical, which can be considered redundant.
The code can be simplified by removing the conditional statement and executing the shared code unconditionally.
However, if this is the intended behavior, there is no reason to use a conditional statement in the first place.