C++ 代码点评. 简单再简单,遵循C和C++ 编程的原则规范,减少代码Bug

Gtest 代码评审: <内部使用 不讲究格式>

 

for 循环问题

Vector <string> t;

....

1. //养成一种习惯 

for(int i = 0 ; i <　t. Size(); i++ ) 错误 修改为 for  (int  i  =  0 ; i  <　t. Size();  ++i ) 

{

业务代码

}

 

在for 循环的代码准则请遵循C的数组概念: i 从0 开始  到 <　一个数结束 [  ) 格式

for(int  i  =  1 ; i  <=　t. Size() -1;  ++i )  不要这么写 , 没有任何好处

避免 用 do {} while 做循环

 

Testconfig.h 文件 

 

#include <iostream>

#include <string>

#include <atmi.h>

#include <ParamTokenizer.h>

#include <exception/TapException.h>

#include <atmiWrapper/ServiceBase.h>

#include <packetWrapper/Recordset.h>

#include <boost/lexical_cast.hpp>

#include <gtest/gtest.h>

#include <boost/algorithm/string.hpp>

 

原理的头包含文件包含太多没有必要的头文件 并且没有按照一定的顺序. 建议如下, 把系统自带库放在最前面, 然后是STL 库 然后是第三方库 然后是自己写的库文件

 

#include <string>

#include <map>

#include <vector>

#include <pugixml.hpp>

#include <boost/shared_ptr.hpp>

//删除没有必要的头文件

 

 

 

定义结构体尽量简单

Typedef struct _serCondition

{   ....

....

} serCondition;

 

Typedef struct serCondition  serCondition_t //习惯性定义

修改为:

struct serCondition

{   ....

....

} 

去掉多余的定义. 如果要定义 请重启一行

Typedef struct serCondition  serCondition_t 习惯是放在定义前面

 

定义类型规范一些 , 

Class  Testconfig 

{

  ....

  ...

private:

void getSerConditionbyNode(pugi::xml_node xmlNode, ServiceDataInfo& servicedatainfo, int flag);

 

    pugi::xml_document m_doc;

map<string, ServiceInfo> m_serviceMap;

}

 

修改为:

Class  Testconfig 

{

  ....

  ...

private:

void getSerConditionbyNode(pugi::xml_node xmlNode, ServiceDataInfo& servicedatainfo, int flag);

 

Private:

    pugi::xml_document m_doc;

 map<string, ServiceInfo> m_serviceMap;

}

 

 

函数参数定义, 尽量少用pugi::xml_node xmlNode 这种模式

void getSerConditionbyNode(pugi::xml_node xmlNode, ServiceDataInfo& servicedatainfo, int flag)

 

修改为

void getSerConditionbyNode(const pugi::xml_node& xmlNode, ServiceDataInfo& servicedatainfo, int flag);

 

 

Testconfig.cpp 文件

 

 

函数

bool TestConfig::getTestByServerName(const string& serverName, ServiceInfo & serviceInfo)

 

修改为

ServiceInfo* TestConfig::getTestByServerName(const string& serverName)

{ //里面的函数体也做一些相应的修改

 

if ( !serverName.empty() )

{

map<string, ServiceInfo>::iterator iter = m_serviceMap.begin();

iter = m_serviceMap.find(serverName);

if (iter != m_serviceMap.end())

{

return &iter->second;

}

}

return NULL;

}

 

 

讲一个比较我们编程比较典型例子:

 

int TestConfig::getPackByServiceName(const string& serviceName, vector<string>& strVe, vector<string>& vExpect)

{

//vector<map<string,string>> veMap;

 

ServiceInfo* serviceinfo = getTestByServerName(serviceName);

if (serviceinfo == NULL)

{

return -1;

}

 

if (serviceinfo->headText.size() == 0)

{

for (int i = 0; i < serviceinfo->serviceData.size(); ++i)

{

strVe.push_back("");

if (!serviceinfo->serviceData[i].caseResult.empty())

{

vExpect.push_back(serviceinfo->serviceData[i].caseResult);

}

else

{

vExpect.push_back("0");

}

}

}

else

{

tr1::unordered_map<string, string> strMap;

for (int i = 0; i < serviceinfo->serviceData.size(); ++i)

{

for (int j = 0; j < serviceinfo->headText.size(); ++j)

{

if (j < serviceinfo->serviceData[i].cellValue.size() )

{

strMap[serviceinfo->headText[j]] = serviceinfo->serviceData[i].cellValue[j];

//strMap[].insert(pair(serviceinfo.serviceName[j],serviceinfo.serviceData[i].CllValue[j]));

}

else

{

strMap[serviceinfo->headText[j]] = "";

}

}

strVe.push_back( RequestPackage::encode(strMap));

strMap.clear();

 

if (!serviceinfo->serviceData[i].caseResult.empty())

{

vExpect.push_back(serviceinfo->serviceData[i].caseResult);

}

else

{

vExpect.push_back("0");

}

}

}

 

return serviceinfo->serviceData.size();

}

 

这段代码运行没有任何问题, 但是如果我们去分析它 , 问题非常多.

 

1, 函数体里面有

ServiceInfo* serviceinfo = getTestByServerName(serviceName);

if (serviceinfo == NULL)

{

return -1;

}

这个代码, 那么 就应该在调用

getPackByServiceName(const string& serviceName, vector<string>& strVe, vector<string>& vExpect);

函数之前获取  ServiceInfo* serviceinfo.

那么代码修改如下

 

getPackByServiceName_1(const ServiceInfo* serviceinfo, vector<string>& strVe, vector<string>& vExpect);

{

for (int i = 0; i < serviceinfo->serviceData.size(); ++i)

{

strVe.push_back("");

if (!serviceinfo->serviceData[i].caseResult.empty())

{

vExpect.push_back(serviceinfo->serviceData[i].caseResult);

}

else

{

vExpect.push_back("0");

}

}

}

 

 

getPackByServiceName_2(const ServiceInfo* serviceinfo, vector<string>& strVe, vector<string>& vExpect);

{

tr1::unordered_map<string, string> strMap;

for (int i = 0; i < serviceinfo->serviceData.size(); ++i)

{

for (int j = 0; j < serviceinfo->headText.size(); ++j)

{

if (j < serviceinfo->serviceData[i].cellValue.size() )

{

strMap[serviceinfo->headText[j]] = serviceinfo->serviceData[i].cellValue[j];

//strMap[].insert(pair(serviceinfo.serviceName[j],serviceinfo.serviceData[i].CllValue[j]));

}

else

{

strMap[serviceinfo->headText[j]] = "";

}

}

strVe.push_back( RequestPackage::encode(strMap));

strMap.clear();

 

if (!serviceinfo->serviceData[i].caseResult.empty())

{

vExpect.push_back(serviceinfo->serviceData[i].caseResult);

}

else

{

vExpect.push_back("0");

}

}

}

 

getPackByServiceName(const string& serviceName, vector<string>& strVe, vector<string>& vExpect);

{

    ServiceInfo* serviceinfo = getTestByServerName(serviceName);

if (serviceinfo == NULL)

{

return -1;

}

 

if (serviceinfo->headText.size() == 0)

{

getPackByServiceName_1(.....);

}

Else

{

getPackByServiceName_2(.....);

}

return serviceinfo->serviceData.size();

 

}

 

呵呵 简单把 肯定不会出bug

 

函数代码 参考代码<testConfig.cpp> 代码

void TestConfig::getSerConditionbyNode(const pugi::xml_node& xmlNode, ServiceDataInfo& servicedatainfo, int flag)  // int flag 的多余的 直接修改为

 

 

 

void TestConfig::getSerConditionbyNode(const pugi::xml_node& xmlNode, ServiceDataInfo& servicedatainfo, std::vector<string> & ServiceData) 

 // std::vector<string> & ServiceData 参数进去就没有必要的逻辑判断.

 

 

下面的代码是不是多余了 

if (serConditionList[i].queOperator.empty())

{

vOperator.push_back("");

}

else

{

vOperator.push_back(serConditionList[i].queOperator);

}

 

其实直接用就可以了

vOperator.push_back(serConditionList[i].queOperator);

 

 

void TestConfig::getPosServiceData(const ServiceDataInfo& serviceDataInfo,

和

void TestConfig::getPreServiceData(const ServiceDataInfo& serviceDataInfo,

以被抽象出来, 雷同的代码尽量抽取出来

 

 

execQueryTest.cpp 问题点

 

很多时候我们在写代码的时候思维是否混乱的

看看这段代码吧 ,

大家对照下面这个函数代码 我抽取一段出来

bool doServiceData(const ServiceDataInfo& servicedatainfo, int flag, string& serverName, string& pack)

 

if (flag == 0)

{

testconfig.getPreServiceData(servicedatainfo, preService, strPack, caseResult, vOperator);

}

else

{

testconfig.getPosServiceData(servicedatainfo, preService, strPack, caseResult, vOperator);

}

 

在我们开始运用 flag 标记的时候 我们需要考虑是否真的需要flag. 其实很多是没有必要的.

如果真的需要 , 请把flag 放在最后一个参数.OK

这里是没有必要的,

 

第一, 既然知道用参数 flag == 0 也就知道调用 testconfig.getPreServiceData函数.

 

第二 如果需要在函数体里面调用, 请用函数指针. 简单明了 减少if 判断. 

 

for (int i = 0; i < preService.size(); ++i)

{

rebuffer = csynCallService(preService[i], strPack[i]);

serverName = preService[i];

pack = strPack[i];

ReplyPackage::decodeErrorCode(rebuffer, errorCode, errorMessage);

    

    if( !preService[i].compare("execQuery"))

{

Recordset recordset;  / /这个定义要考后

if (errorCode != "0")  //这个判断是否应该提前

{

return false;

}

else

{

ReplyPackage::decodeFirstDataset(rebuffer, recordset);

if (!toQueOperator(vOperator[i], recordset.getFieldValue(0, 0), caseResult[i])) 

           {

return false;

}

}

}

else

{

if (errorCode != caseResult[i])

{

return false;

}

}

}

Return true;

 

 

代码修改如下.

for (int i = 0; i < preService.size(); ++i)

{

rebuffer = csynCallService(preService[i], strPack[i]);

serverName = preService[i];

pack = strPack[i];

ReplyPackage::decodeErrorCode(rebuffer, errorCode, errorMessage);

 

if (errorCode != caseResult[i])

{

return false;

}

    

    if( errorCode == "0" && !preService[i].compare("execQuery"))

{

Recordset recordset; 

        ReplyPackage::decodeFirstDataset(rebuffer, recordset);

if (!toQueOperator(vOperator[i], recordset.getFieldValue(0, 0), caseResult[i]))

          {

return false;

}

}

}

Return true;

 

代码逻辑应该清楚一些了吧

 

函数变量定义原则. 只有在需要定义的时候才定义, 不要提前定义.

 

mainTest.cpp

 

大家查看源代码

TEST_F(maintest, tests)

 

以下是我重写的代码:

重构原则, 抽象出相同的代码. 

 

void showServiceData(vector<serCondition>& ServiceData)

{

for (int x = 0; x < ServiceData.size(); ++x)

{

cout << "posServiceData:" << endl;

 

for_each(ServiceData[x].headText.begin(),ServiceData[x].headText.end(),ostream_iterator<string>(cout, " "));

for_each(ServiceData[x].cellValue.begin(),ServiceData[x].cellValue.end(),ostream_iterator<string>(cout, " "));

}

}

 

void showService(vector<string>& services,vector<string>&caseResult ,vector<string>&pack,vector<string>&vOperator )

{

for (int posIndex = 0; posIndex < services.size(); posIndex++)

{

cout << "serviceName:" << services[i] << endl;

cout << "pack:" << pack[i] << endl;

cout << ":" << caseResult[i] << endl;

cout << "Operator:" << vOperator[i] << endl;

}

}

 

TEST_F(maintest, tests)

{

 

//这段代码需要拆分每一个并列的for 或者if 语句 基本是两件事情 ,需要用不同的函数进行细分

vector<string> pack;

vector<string> preService;

vector<string> caseResult;

vector<string> vOperator;

 

TestConfig testconfig;

testconfig.init();

string serverName = "login";

ServiceInfo* serviceinfo = testconfig.getTestByServerName(serverName);

 

for (int i = 0; i < serviceinfo->serviceData.size(); ++i)

{

for (int j = 0; j < serviceinfo->serviceData[i].cellValue.size(); ++j)

{

cout << serviceinfo->serviceData[i].cellValue[j] << endl;

}

 

showServiceData(serviceinfo->serviceData[i].posServiceData);

cout << endl;

testconfig.getPosServiceData(serviceinfo->serviceData[i], preService, pack, caseResult, vOperator);

showService(serviceinfo->serviceData[i], caseResult, pack, vOperator );

 

 

showServiceData(serviceinfo->serviceData[i].preServiceData);

cout << endl;

testconfig.getPosServiceData(serviceinfo->serviceData[i], preService, pack, caseResult, vOperator);

showService(serviceinfo->serviceData[i], caseResult, pack, vOperator );

cout << endl << endl << endl << endl << endl;

 

testconfig.getPreServiceData(serviceinfo->serviceData[i], preService, pack, caseResult, vOperator);

showService(serviceinfo->serviceData[i], caseResult, pack, vOperator );

cout << endl;

}

 

vector<string> expect;

vector<string> strVe;

int rowTest = testconfig.getPackByServiceName(serverName, strVe, expect);

for (int i = 0; i < rowTest; ++i)

{

cout << strVe[i] << endl;

cout << expect[i] << endl;

}

}

转载于:https://www.cnblogs.com/willyzhao/archive/2012/07/20/2601040.html