Fork me on GitHub

团队C++代码走查小结(设计篇)

the summary of C++ code review(2)

Posted by Kaelzhang on April 15, 2017

code review

团队C++代码走查小结(设计篇)

本文内容源于团队的每日代码走查活动,对照Clean Code基本原则和项目C++编码规范将项目代码中实际出现的一些问题进行提炼归纳,以便经验传播、避免问题反复出现。 PS:由于信息安全已对示例代码进行去业务处理。

物理设计

冗余的头文件

// #include "haed1andhaed2.h"
#include "haed1.h"

Haed1 haed;

所包含的头文件已经被包含在其他头文件中,无需重复包含。

范围过大的头文件

// #include "all.h"
#include "part.h"

Part part;

所包含的头文件范围大于实际使用的范围,需要进一步缩小依赖范围。

重复

逻辑上的重复

项目中存在大量类似但又不完全一样的代码,可以通过OO的多态或者函数式编程的方法来消除。 示例:

void FakeSystem::wait(string session, string key, const EventId& eventId)
{
    reschedure();
    try{
        auto msg = RECV_MSG_QUEUE.pop(session, key, eventId);

        instKey = msg.instKey();
        session = msg.session();

    }catch(const Exception e)
    {
        RECV_MSG_QUEUE.dump();
        throw e;
    }
}
void FakeSystem::wait(const EventId& eventId)
{
    reschedure();
    try{
        auto msg = RECV_MSG_QUEUE.pop(eventId, *this);

        instKey = msg.instKey();
        session = msg.session();
    }catch(const Exception e)
    {
        RECV_MSG_QUEUE.dump();
        throw e;
    }
}

可以尝试通过lambda表达式来消除重复的逻辑:

void FakeSystem::wait(string session, string key, const EventId& eventId)
{
    auto pop = [=](){
        return RECV_MSG_QUEUE.pop(session, key, eventId);
    };

    waitMsg(eventId, pop);
}
void FakeSystem::wait(const EventId& eventId)
{
    auto pop = [=](){
        return RECV_MSG_QUEUE.pop(eventId, *this);
    };

    waitMsg(eventId, asserter, pop);
}
void FakeSystem::waitMsg(const EventId& eventId, const std::function<Message()>& pop)
{
    reschedure();
    try{
        auto msg = pop();

        instKey = msg.instKey();
        session = msg.session();
    }catch(const Exception e)
    {
        RECV_MSG_QUEUE.dump();
        throw e;
    }
}

效率提升

不必要的对象赋值

CellData lmCellInfoInterface(WORD32 Id)
{
	XXXData xxxData = XXXRepository::getInstance().getXXXInfo(Id);

	return xxxData;
}

直接return即可。

右值引用

使用右值引用实现移动语义减少拷贝赋值。 示例:

    XXXInfo xxxInfo(&info);
    xxxInfo.update(val);

    auto var = map.find(info.id);
    DCM_ASSERT_TRUE(var == map.end());

    map[info.id] = std::move(xxxInfo)

无序容器

传统的map和set是有序的容器,在插入元素时会对容器进行自动排序,在非排序的场景下使用无序容器unordered_map和unordered_set会更加高效。 示例:

DEF_INTERFACE(If, 0x170323)
{
    Status save();
    Status update(val);
    Info delete(val);

    Status accept(Visitor&) const;

private:
	std::unordered_map<ID, Info> map;
}

传统的map的实现使用的是红黑树,而无序map则是使用散列表实现。

技巧

auto类型推导的使用

通过auto关键字的使用,隐藏不必要的冗长信息。

using ns::IdQueryRequest;
using ns::Id;

Status IdQueryRole::build(ns::Message& msg)
{
    IdQueryRequest* req = msg.id_query_request();
    ASSERT_VALID_PTR(req);

    Id* id = req->getId();
    ASSERT_VALID_PTR(id);

    id->set_id(ROLE(InfoField)->id());
    id->set_val(ROLE(IdField)->value());

    return SUCCESS;
}
~~using ns::IdQueryRequest;
using ns::Id;~~

Status IdQueryRole::build(ns::Message& msg)
{
    auto req = msg.id_query_request();
    ASSERT_VALID_PTR(req);

    auto id = req->getId();
    ASSERT_VALID_PTR(id);

    id->set_id(ROLE(InfoField)->id());
    id->set_val(ROLE(IdField)->value());

    return SUCCESS;
}

基于范围的for循环

C++11基于范围的for以统一、简洁的方式来遍历容器和数组。

    const Message pop(const EventId& eventId, const FakeSystem& sys)
    {
        typedef std::list<Message>::iterator Iter;
        for (Iter i=msgs.begin(); i != msgs.end(); ++i)
        {
            if ((i->messageId() == eventId) && (sys.expect(*i)))
            {
                Message msg = *i;
                msgs.erase(i);
                return msg;
            }
        }
        onFail(string(), string(), eventId);
    }

改为:

const Message pop(const EventId& eventId, const FakeSystem& sys)
    {
        for (auto& msg: msgs)
        {
            if (msg.messageId() == eventId) && (sys.expect(msg))
            {
                Message out = *msg;
                msgs.erase(msg);
                return out;
            }
        }
        onFail(string(), string(), eventId);
    }

或者结合lambda:

    const Message pop(const EventId& eventId, const FakeSystem& sys)
    {
        auto msg = find_if(msgs.begin(), msgs.end(), [&](Message& msg){
            return (msg.messageId() == eventId) && (sys.expect(msg));
        });

        if(out != msgs.end())
        {
            Message out = *msg;
            msgs.erase(msg);
            return out;
        }

        onFail(string(), string(), eventId);
    }

  • 版权声明: 本博客所有文章除特别声明外,均采用 CC BY-NC-SA 3.0 许可协议。转载请注明出处!