I - 代码异味
此部分为可能产生问题或容易产生问题的代码,或者冗余代码等。
1.1 - C-style pointer casting
C 风格的指针转换
代码示例
char* p =(char*) str.c_str();
应使用
char* p =const_cast<char*> str.c_str();
应尽量使用 C++ 提供的四种转换,C 风格的转换缺少严谨性与识别度,C++ 在转换错误时会有错误提醒。
- static_cast 用于层次结构内的指针和引用向上转换,即 派生类到基类的转换,和其他数据类型以及指针的相互转换。
-
dynamic_cast 用于层次结构内的指针和引用的向下转换,在转换时有运行时类型检查,转换错误会返回
nullptr
。 - const_cast 用于去除指针、引用、对象的常属性,转换为非常量的指针、引用、对象。
- reinterpret_cast 可以把指针转为为数据,也可以把数据转换为指针,注:非必要时避免使用。
1.2 - Clarify calculation precedence for 'x1' and 'x2'
举例:Clarify calculation precedence for '&' and '?',明确运算符 '&' 和 '?' 的优先级
UpdateUndoRedo(params, Flags::Undo & undoRedo ?true:false, Flags::Redo & undoRedo ?true:false);
具有相同优先级的操作符应使用小括号明确优先级,修改为:
UpdateUndoRedo(params,bool(Flags::Undo & undoRedo),bool(Flags::Redo & undoRedo));
1.3 - Class 'XXX' does not have an operator= which is recommended since it has dynamic memory/resource allocation(s).
举例:Class 'HttpServer' does not have an operator= which is recommended since it has dynamic memory/resource allocation(s).
类 'HttpServer' 包含动态分配的内存/资源,但是没有赋值操作符重载
HttpServer(){
//...
m_socket =newHttpSocket(/* args */);}
需要显式实现拷贝构造和赋值操作符重载,避免浅拷贝和双释放造成的错误。举例修改如下:
HttpServer(const HttpServer& rhs)=delete;
HttpServer&operator=(const HttpServer& rhs)=delete;
未使用拷贝构造和赋值操作符重载的情况下可以禁用。
1.4 - Class 'XXX' has a constructor with 1 argument that is not explicit
举例:Class 'Widget' has a constructor with 1 argument that is not explicit.
类 'Widget' 有一个单实参数的构造函数,但是未标识 explicit 。
单实参的构造函数,未标识 explicit 会发生隐式转换,如参数非 int 但可以使用 int 类型隐式转换并且构造,可能出现构造错误。
classWidget{
public:Widget(const std::string& name,bool isTool =true);//...};
因为包含默认参数,因此可以使用一个实参去构造,需要添加 explicit 关键字
classWidget{
public:explicitWidget(const std::string& name,bool isTool =true);//...};
1.5 - Condition '-1 != XXX' is always true
举例: Condition '-1 != (',')
' is always true.
条件判断 '-1 != (',')' 恒为真。
std::string& line = lines[i];if(-1!= ("X")){
vec.push_back(line);}
虽然不规范,但目前还没遇到错误,std::string
的 find
函数在查找不到时返回 std::string::npos
为 size_t
类型,非负数,与 -1 比较时,会向上转换成一种公共类型。建议修改 -1 为 std::string::npos
。
std::string& line = lines[i];if(std::string::npos != ("X")){
vec.push_back(line);}
1.6 - Condition 'XXX' is always true
举例:Condition 'checkFlag' is always true.
条件 'checkFlag' 恒为真
if(!checkFlag){
// do some thing}elseif(ConditionB && checkFlag){
// ... do some thing else}
在 else if
中的 checkFlag 应恒为真,由于 if
分支已经判断的不为真的情况。应将冗余的判断删除
if(!checkFlag){
// do some thing}elseif(ConditionB)// 删除 checkFlag{
// ... do some thing else}
1.7 - Function 'XXX' argument X names different: declaration 'XX1' definition 'XX2'
举例:Function 'CheckRemoteConnection' argument 3 names different: declaration 'remoteType' definition 'type'.
函数 ‘CheckRemoteConnection’ 第三个参数名称在函数声明与定义中不一致
intCheckRemoteConnection(const std::string& ip,unsignedshort port,int remoteType);//...int Server::CheckRemoteConnection(const std::string& ip,unsignedshort port,int type){
//...}
应修改为相同名称,避免差异较大时实现代码与声明中的注释不一致或其他的情况。
intCheckRemoteConnection(const std::string& ip,unsignedshort port,int remoteType);//...int Server::CheckRemoteConnection(const std::string& ip,unsignedshort port,int remoteType){
//...}
1.8 - Local variable 'XXX' shadows outer function
局部变量与外部作用域函数同名。
应修改局部变量名称,避免出错。
1.9 - Local variable 'XXX' shadows outer variable
举例:Local variable 'pRemote' shadows outer variable.
局部变量 ‘pRemote’ 覆盖外作用域同名变量。
Remote* pRemote =GetRemoteHead();if(pRemote){
//...}//...for(auto param : m_paramlist){
Remote* pRemote =GetRemote(param);//...}
应避免局部变量覆盖外部作用域同名变量,或可能出现操作错误
Remote* pRemote =GetRemoteHead();if(pRemote){
//...}//...for(auto param : m_paramlist){
Remote* pRemoteLocal =GetRemote(param);//...}
1.10 - Member variable 'XXX' is in the wrong place in the initializer list
举例:Member variable 'm_hDevice' is in the wrong place in the initializer list.
类成员变量 ‘m_hDevice’ 声明的顺序与构造函数初始化列表中定义的顺序不一致。
示例代码:
//...private:
HANDLE m_hDevice;//...public:bool m_selected;// ...
顺序达成一致即可,调整构造函数初始化列表或者调整声明中的顺序
//...public:bool m_selected;//...private:
HANDLE m_hDevice;// ...
类成员的初始化顺序是按照类内数据成员的声明顺序进行初始化的,这样可以减少不必要的开销,类不必为每一个对象进行跟踪初始化数据成员,只需要按照类中的声明顺序初始化即可。(析构函数也会出现类似的情况)
1.11 - Member variable 'XXX' is not initialized in the constructor
举例:Memer variable 'CRemote::m_id' is not initialized in the constructor.
成员变量 'CRemote::m_id' 未在构造函数中初始化。
classCRemote:/*...*/{
public:CRemote(const Params& param);CRemote(const std::string& ip,unsignedshort port);//...int m_id;};
可能存在多个构造函数,而在开发中某个构造函数疏漏了一个或几个成员变量的初始化,最好在声明处赋上默认值,避免成员变量未初始化,造成未知的错误。
classCRemote:/*...*/{
public:CRemote(const Params& param);CRemote(const std::string& ip,unsignedshort port);//...int m_id {
DEFAULT_ID };};
1.12 - Passing the result of c_str() to a function that takes std::string as argument no.1 is slow and redundant
函数第一个参数为 std::string,调用时传递 c_str(),低效且冗余
CheckList(remotename.c_str(),/* args */);
可能为编写时的疏忽,或接口后期更新。需要删除冗余的 c_str()
CheckList(remotename,/* args */);
1.13 - The if condition is the same as the previous if condition
if 判断条件与前一个 if 相同
bool success =GetRunState();if(success){
//...}//...if(true== success)
if 条件重复,检查是否可以合并
1.14 - The statement 'XX1' is logically equivalent to 'XX2'
举例:The statement 'if (m_name != newname) m_name = newname;
' is logically equivalent to 'm_name = newname
'
语句 'if (m_name != newname) m_name = newname;
' 与 'm_name = newname
' 逻辑等价
if(m_name != newname){
m_name = newname;}
示例修改
m_name = newname;
1.15 - Unused private function : 'XXX'
未使用的函数,若为弃用的函数,则可删除废弃代码,如果为备用函数则可忽略掉本条检视问题。
1.16 - Unused variable : 'XXX'
未使用的变量,可以删除。
1.17 - Variable 'XXX' is assigned value that is never used
举例:Variable 'index' is assigned value that is never used.
变量 index 被赋值,但此值从未使用。
int index =-1;//...
index =GetRemoteIndex(/* args */);
变量定义与赋值可在同一处。
int index =GetRemoteIndex(/* args */);
1.18 - Variable 'XXX' is reassigned a value before old one has been used
与前一条类似。
II - 代码错误
此部分为明确的代码错误。
2.1 - Checking if unsigned expression 'XXX' is less than zero
检查无符号的表达式小于 0 ,举例
Checking if unsigned expression '()' less than zero
std::vector<int> vec;//...if(()<=0)
std::vector
的 size()
方法的返回值为 size_t
类型,x64 的定义为
typedefunsignedlonglong size_t;
无符号类型恒为非负数,应修改为
if(()==0)// 或if(!())
建议修改为后者,由于 empty()
一般为常数时间复杂度。
2.2 - Access of moved variable 'XXX'
访问了移动后的变量。举例
Acess of moved variable 'graph'.
vec.push_back(std::move(graph));
();
变量在 std::move 之后资源的权限已转移,内存已不可使用,不能再次使用被移动后的变量,应去除使用
vec.push_back(std::move(graph));
2.3 - Class 'XXX' is not safe, destructor throws exception
举例: Class 'Strategy' is not safe, destructor throws exception.
类 Strategy 在析构函数中抛出异常不安全。
应尽量在问题处抛出异常,而不是在析构处。
2.4 - Class 'XXX' is unsafe, 'XXX:xxx' can leak by wrong usage
举例: Class 'HttpSocket' is unsafe, 'HttpSocket:m_socket' can leak by wrong usage.
类 'HttpSocket' 不安全,成员变量 'HttpSocket:m_socket' 在使用错误时会造成内存泄漏
classHttpSocket:public/*...*/{
// ...private:
Socket* m_socket {
nullptr};}
m_socket 为动态分配的指针,但未对其进行释放。
HttpSocket::~HttpSocket(){
// ...if(m_socket){
delete m_socket;
m_socket =nullptr;}}
2.5 - Consecutive return, break, continue, goto or throw statements are unnecessary
不必要的连续跳转语句 return, break, continue, goto 或 throw
示例代码
{
returnUpdateComponent(/*args*/);returntrue;}
连续的跳转语句只会执行第一个,return true 永远执行不到,需要删除
{
returnUpdateComponent(/*args*/);}
2.6 - Statements following return, break, continue, goto, or throw will never be executed
跳转语句 return, break, continue, goto 或 throw 后边的多个语句永远不会执行
代码示例:
{
//...returntrue;{
//...}}
应检查删除 return true 后不会执行的代码
{
//...returntrue;}
初级错误,无用代码,删除前需要找对应的开发确认。
2.7 - Either the condition 'XXX' is redundant or there is possible null pointer derefence: XXX
举例:Either the condition '!arr' is redundant or there is possible null pointer dereference: arr.
指针非空条件判断 '!arr' 冗余或存在空指针解引用的可能。
代码示例:
arr =newint[num];if(!arr){
PrintError();}
new
操作符分配失败,会抛出异常,无法使用指针非空来判断,修改示例:
arr =newint[num];
删除指针非空的检查
2.8 - Exception should be caught by reference
异常捕获应使用引用
代码示例:
try{
// some code ...}catch(MyExceptionBase e){
// exception handle ... }
- 拷贝构造会产生额外开销
- 以基类值捕获会产生对象切割,派生对象只能保留基类部分数据
应修改为引用,示例修改try{ // some code... }catch(MyExceptionBase& e){ // exception handle ...}
2.9 - Found duplicate branches for 'XX' and 'XX'
举例:Found duplicate branches for 'if' and 'else'.
if 和 else 分支中代码重复,代码示例:
if(remoteCount <0){
PrintError();}else{
PrintError();}
应删除冗余的代码,示例修改:
if(remoteCount <0){
PrintError();}
2.10 - Opposite expression on both sides of 'XXX'
举例:Oppsite expression on both side of '&&'.
在逻辑与两侧的表达式相反,彼此互补。
代码示例:
if(!valid && valid)
需要检查修改,此处为逻辑错误,if 分支总是不会执行。
2.11 - Possible null pointer derefence : XXX
举例:Possible null pointer derefence : p.
指针变量 'p' ,可能的空指针解引用。
sz = p->GetSize();
指针在使用前需要判断是否为空
if(!p){
PrintError("null pointer");returnfalse;}
sz = p->GetSize();
2.12 - Resource leak : XXX
举例:Resource leak : fp.
fp 资源泄漏
fp =fopen(path.c_str(),"a+");
fp 文件指针未释放,需要关闭文件描述符
fp =fopen(path.c_str(),"a+");//...fclose(fp);// 关闭文件描述符
2.13 - The comparison 'XXX' is always true because 'XXX1' and 'XXX2' represent the same value
举例:The comparison 'name == ""
' is always true because 'name
' and '""
' represent the same value.
比较 'name == ""
' 恒为真, 由于 'name
' 与 '""
' 表示相同的值。
std::string name ="";if(name =="")
若非有意设置固定值,应为代码错误。
2.14 - Uninitialized variable : 'XXX'
未初始化的变量,变量最好在声明时初始化。
2.15 - Virtual function 'XXX' is called from constructor 'XX1' at line 'XX2'. Dynamic binding is not used
举例:Virtual function 'UpdateStatus' is called from constructor 'Remote' at line '42'. Dynamic binding is not used.
虚函数 UpdateStatus 在 42 行,构造函数 Remote 中调用。动态绑定未使用。
Remote::Remote(/* args */){
//...UpdateStatus();}
构造函数中调用虚函数无法使用动态多态,析构函数中同样。
可以将实现提到一个函数中,构造函数中调用此函数,原虚函数为此函数的封装,也调用这个接口。示例修改:
Remote::Remote(){
//...UpdateStatusImpl();}//...void Remote::UpdateStatusImpl(){
// 原 UpdateStatus 实现}void Remote::UpdateStatus(){
UpdateStatusImpl();}
此种修改不影响动态多态
III - 代码优化
此部分为可优化的代码,去除冗余的判断,提高代码执行效率等。
3.1 - Consider using std::accumulate algorithm instead of a raw loop
考虑使用标准库 accumulate
算法替代使用原生循环
示例代码:
for(auto it : strvec){
namelist += _SEPARATOR_ + it;}
参考修改
if(!()){
namelist = std::accumulate(std::next(()), (), strvec[0],[](std::string str, std::string& elem){
return str + _SEPARATOR_ + elem;});}
这种实现避免第一个字符为分隔字符的情况。
为了使用 std::accumulate , 包含的头文件不是 <algorithm>
而是 <numeric>
。
3.2 - Consider using std::any_of algorithm instead of a raw loop
考虑使用 std::any_of
算法替代原生的循环
示例代码:
for(auto e : str){
returnisdigit(e);}
修改示例
return std::any_of((), (),[](char c)->bool{
returnisdigit(c);});
lambda 表达式 -> bool
可省略,通过返回值编译器可以自行推断。 std::any_of 集合中只要有一个元素符合条件即返回 true,集合为空或者全部元素为 false 则返回 false。声明为:
template<class_InIt,class_Pr>inlineboolany_of(const _InIt _First,const _InIt _Last, _Pr _Pred);
3.3 - Consider using std::copy algorithm instead of a raw loop
考虑使用 std::copy
算法替代原生的循环
示例代码:
std::vector<Material*> vec;for(auto materialp : m_materials){
vec.push_back(materialp);}
修改示例:
std::vector<Materials*> vec;
std::copy(m_materials.begin(), m_materials.end(), ());
标准库算法通常比原生循环有更好的可读性,执行效率和安全性。
3.4 - Consider using std::copy_if algorithm instead of a raw loop
考虑使用标准库算法 std::copy_if
替代原生循环
代码示例:
std::vector<GenericComponent*> components;//...for(auto comp : components){
if(!comp->GetType().compare(type)){
vecRt.push_back(comp);}}
修改示例:
std::copy_if((), (), std::back_inserter(vecRt),[&type](GenericComponent* gcp){
return(0== gcp->GetType().compare(type));});
标准库算法通常比原生循环有更好的可读性,执行效率和安全性。
3.5 - Consider using std::find_if algorithm instead of a raw loop
考虑使用标准库算法 std::find_if
替代原生循环
代码示例:
for(/*...*/){
if( == num){
return ;}}
修改示例
auto iter = std::find_if((), (),[num](const Component& comp){
return( == num);});if(()!= iter){
return(*iter).model;}
3.6 - Consider using std::replace_if algorithm instead of a raw loop
考虑使用标准库算法 std::replace_if
替代原生循环
代码示例:
for(std::string& elem : strvec){
if(0== (_SEPARATOR_)){
elem ="";}}
修改示例
std::replace_if((), (),[](const std::string& elem){
return _SEPARATOR_ == elem;});
3.7 - Consider using std::transform algorithm instead of a raw loop
考虑使用标准库 std::transform
算法替代原生循环
代码示例:
std::vector<std::string> vec;for(auto comp : m_comps){
vec.push_back(->GetComponentName());}
示例修改:
std::transform(m_comps.begin(), m_comps.end(), std::back_inserter(vec),[](std::pair<int, GenericComponent*> pr){
return ->GetComponentName();});
3.8 - Function parameter 'XXX' should be passed by const reference
举例:Function parameter 'ip' should be passed by const reference.
函数参数 'ip' 应该传递常量引用
intPingRemote(std::string ip);
- 传递引用为避免在函数体中额外的拷贝构造和析构函数的开销,提高执行效率。
- 常量引用可以匹配左值和右值,此处为可以匹配字符串字面值。
- 若参数在函数体中不修改,标识 const 在意外修改时编译器会生成错误。
示例修改:
intPingRemote(const std::string& ip);
3.9 - Ineffective call of function 'substr' because a prefix of the string is assigned to itself. Use resize() or pop_back() instead
'substr' 函数调用低效,字符串首开始的子串赋值给自身,可使用 resize() 或 pop_back() 替代
代码示例:
// 1. 去掉最后一个字符
line = (0, ()-1);// 2. 截取符号之前的字符串
size_t index = ('.');if(std::string::npos != index){
line = (0, index);}
示例修改
// 1. 去掉最后一个字符if(!())
line.pop_back();// 2. 截取符号之前的字符串
size_t index = ('.');if(std::string::npos != index){
(index);}
3.10 - Prefer prefix ++/-- operators for non-primitive types
对于非基础类型建议使用前置 ++
/ --
,前置自增自减,效率更高。
3.11 - Redundant condition: XXX. 'XX1' is equivalent to 'XX2'
举例:Redundant condition: p, '!p || (p && 1 == ())
' is equivalent to '!p || 1 == ()
'。
冗余的条件判断 p , '!p || (p && 1 == ())
' 可等价修改为 '!p || 1 == ()
'。
if(!p ||(p &&1== ()))
||
在第一个条件成立的情况下会短路,第二个条件不会执行。只有前者不成立时后者才会执行。
换而言之, p 为空,会跳过第二个判断。只有第一个条件为假时,才会执行第二个,也就是 p 为非空时才会执行第二部分的 p && 1 == ()
, p 非空已经判断过了,不需要重复判断。
if(!p ||1== ())
3.12 - Technically the member function 'XXX' can be const
举例:Technically the member function 'Remote::GetStatus()' can be const.
未改变成员变量的函数应声明未常函数
intGetStatus(){
return m_status;}
const 修饰函数时写在参数和函数体之间,可以避免类属性被意外修改。
intGetStatus()const{
return m_status;}
注:常量对象无法调用非 const 修饰的函数。
3.13 - Technically the member function 'XXX' can be static (but you may consider moving to unnamed namespace)
技术上来说未访问非静态成员变量的函数可以声明为 static
静态成员函数调用时可以不必构造对象。
3.14 - The function 'XXX' overrides a function in a base class but is not marked with a 'override' specifier.
举例:The function 'TellPosition' overrides a function in a base class but is not marked with 'override' specifer.
函数 ‘TellPosition’ 重写基类中的虚函数但未标识 'override' 关键字
virtual Pos TellPosition();
为保证编译器检查,防止编码错误导致与基类方法不一致,以至于无法正确实现多态。另外,标注 override 关键字,还可以在基类方法修改后,能够借助编译器及时且全面的在派生类中进行更新。
virtual Pos TellPosition() override;
3.15 - The scope of the variable 'XXX' can be reduced
举例:The scope of the variable 'prefix' can be reduced.
变量 prefix 的作用域可以缩减
std::string prefix ="args";if(condition){
//...SaveConfigFile(prefix + key, value);}else{
//...SaveConfigFile(key, value);}
当前变量作用域过大,缩小作用域可以在程序不运行到特定分支时,不为变量分配内存。
if(condition){
std::string prefix ="args";//...SaveConfigFile(prefix + key, value);}else{
//...SaveConfigFile(key, value);}
3.16 - Variable 'XXX' is assigned in constructor body. Consider performing initialization in initializer list
举例:Variable 'm_suffixlist' is assigned in constructor body. Consider performing initialisation in initializer list.
变量 m_suffixlist 在构造函数中赋值,考虑在构造函数初始化列表中初始化
FileManip::FileManip(){
m_suffixlist ={
".cxx",".cpp",".hpp",".cc"};//...}
类类型的变量,或者叫聚合类型变量,初始化建议放在构造函数初始化列表,这种情况下 只调用一次构造。而放置在构造函数中,则会首先调用默认构造,再调用一次赋值。前者效率更高
FileManip::FileManip(): m_suffixlist {
".cxx",".cpp",".hpp",".cc"}{
//...}