陈硕 (giantchen_AT_gmail)
Blog.csdn.net/Solstice
版本管理(version controlling)是每个程序员的基本技能,C++ 程序员也不例外。版本管理的基本功能之一是追踪代码变化,让你能清楚地知道代码是如何一步步变成现在的这个样子,以及每次 check-in 都具体改动了哪些内部。无论是传统的集中式版本管理工具,如 Subversion,还是新型的分布式管理工具,如 Git/Hg,比较两个版本(revision)的差异都是其基本功能,即俗称“做一下 diff”。
diff 的输出是个窥孔(peephole),它的上下文有限(diff –u 默认显示前后 3 行)。在做 code review 的时候,如果能凭这“一孔之见”就能发现代码改动有问题,那就再好也不过了。
C 和 C++ 都是自由格式的语言,代码中的换行符被当做 white space 来对待。(当然,我们说的是预处理(preprocess)之后的情况)。对编译器来说一模一样的代码可以有多种写法,比如
foo(1, 2, 3, 4);
和
foo(1,
2,
3,
4);
词法分析的结果是一样的,语意也完全一样。
对人来说,这两种写法读起来不一样,对与版本管理工具来说,同样功能的修改造成的差异(diff)也往往不一样。所谓“有利于版本管理”,就是指在代码中合理使用换行符,对 diff 工具友好,让 diff 的结果清晰明了地表达代码的改动。(diff 一般以行为单位,也可以以单词为单位,本文只考虑最常见的 diff by lines。)
这里举一些例子。
对 diff 友好的代码格式
1. 多行注释也用 //,不用 /* */
Scott Meyers 写的《Effective C++》第二版第 4 条建议使用 C++ 风格,我这里为他补充一条理由:对 diff 友好。比如,我要注释一大段代码(其实这不是个好的做法,但是在实践中有时会遇到),如果用 /* */,那么得到的 diff 是:
diff --git a/examples/asio/tutorial/timer5/timer.cc b/examples/asio/tutorial/timer5/timer.cc --- a/examples/asio/tutorial/timer5/timer.cc +++ b/examples/asio/tutorial/timer5/timer.cc @@ -18,6 +18,7 @@ class Printer : boost::noncopyable loop2_->runAfter(1, boost::bind(&Printer::print2, this)); } + /* ~Printer() { std::cout << "Final count is " << count_ << "\n"; @@ -38,6 +39,7 @@ class Printer : boost::noncopyable loop1_->quit(); } } + */ void print2() {
从这样的 diff output 能看出注释了哪些代码吗?
如果用 //,结果会清晰很多:
diff --git a/examples/asio/tutorial/timer5/timer.cc b/examples/asio/tutorial/timer5/timer.cc --- a/examples/asio/tutorial/timer5/timer.cc +++ b/examples/asio/tutorial/timer5/timer.cc @@ -18,26 +18,26 @@ class Printer : boost::noncopyable loop2_->runAfter(1, boost::bind(&Printer::print2, this)); } - ~Printer() - { - std::cout << "Final count is " << count_ << "\n"; - } + // ~Printer() + // { + // std::cout << "Final count is " << count_ << "\n"; + // } - void print1() - { - muduo::MutexLockGuard lock(mutex_); - if (count_ < 10) - { - std::cout << "Timer 1: " << count_ << "\n"; - ++count_; - - loop1_->runAfter(1, boost::bind(&Printer::print1, this)); - } - else - { - loop1_->quit(); - } - } + // void print1() + // { + // muduo::MutexLockGuard lock(mutex_); + // if (count_ < 10) + // { + // std::cout << "Timer 1: " << count_ << "\n"; + // ++count_; + // + // loop1_->runAfter(1, boost::bind(&Printer::print1, this)); + // } + // else + // { + // loop1_->quit(); + // } + // } void print2() {
同样的道理,取消注释的时候 // 也比 /* */ 更清晰。
另外,如果用 /* */ 来做多行注释,从 diff 不一定能看出来你是在修改代码还是修改注释。比如以下 diff 似乎修改了 muduo::EventLoop::runAfter 的调用参数:
diff --git a/examples/asio/tutorial/timer5/timer.cc b/examples/asio/tutorial/timer5/timer.cc --- a/examples/asio/tutorial/timer5/timer.cc +++ b/examples/asio/tutorial/timer5/timer.cc @@ -32,7 +32,7 @@ class Printer : boost::noncopyable std::cout << "Timer 1: " << count_ << "\n"; ++count_; - loop1_->runAfter(1, boost::bind(&Printer::print1, this)); + loop1_->runAfter(2, boost::bind(&Printer::print1, this)); } else {
其实这个修改发生在注释里边 (要增加上下文才能看到, diff -U 20,多一道手续,降低了工作效率),对代码行为没有影响:
diff --git a/examples/asio/tutorial/timer5/timer.cc b/examples/asio/tutorial/timer5/timer.cc --- a/examples/asio/tutorial/timer5/timer.cc +++ b/examples/asio/tutorial/timer5/timer.cc @@ -20,31 +20,31 @@ class Printer : boost::noncopyable /* ~Printer() { std::cout << "Final count is " << count_ << "\n"; } void print1() { muduo::MutexLockGuard lock(mutex_); if (count_ < 10) { std::cout << "Timer 1: " << count_ << "\n"; ++count_; - loop1_->runAfter(1, boost::bind(&Printer::print1, this)); + loop1_->runAfter(2, boost::bind(&Printer::print1, this)); } else { loop1_->quit(); } } */ void print2() { muduo::MutexLockGuard lock(mutex_); if (count_ < 10) { std::cout << "Timer 2: " << count_ << "\n"; ++count_;
总之,不要用 /* */ 来注释多行代码。
或许是时过境迁,大家都在用 // 注释了,《Effective C++》第三版去掉了这一条建议。
2. 局部变量与成员变量的定义
基本原则是,一行代码只定义一个变量,比如
double x;
double y;
将来代码增加一个 double z 的时候,diff 输出一眼就能看出改了什么:
@@ -63,6 +63,7 @@ private:
int count_;
double x;
double y;
+ double z;
};
int main()
如果把 x 和 y 写在一行,diff 的输出就得多看几眼才知道。
@@ -61,7 +61,7 @@ private: muduo::net::EventLoop* loop1_; muduo::net::EventLoop* loop2_; int count_; - double x, y; + double x, y, z; }; int main()
所以,一行只定义一个变量更利于版本管理。同样的道理适用于 enum 成员的定义,数组的初始化列表等等。
3. 函数声明中的参数
如果函数的参数大于 3 个,那么在逗号后面换行,这样每个参数占一行,便于 diff。以 muduo::net::TcpClient 为例:
class TcpClient : boost::noncopyable
{
public:
TcpClient(EventLoop* loop,
const InetAddress& serverAddr,
const string& name);
如果将来 TcpClient 的构造函数增加或修改一个参数,那么很容易从 diff 看出来。这恐怕比在一行长代码里数逗号要高效一些。
4. 函数调用时的参数
在函数调用的时候,如果参数大于 3 个,那么把实参分行写。以 muduo::net::EPollPoller 为例:
Timestamp EPollPoller::poll(int timeoutMs, ChannelList* activeChannels)
{
int numEvents = ::epoll_wait(epollfd_,
&*events_.begin(),
static_cast<int>(events_.size()),
timeoutMs);
Timestamp now(Timestamp::now());
这样一来,如果将来重构引入了一个新参数(好吧,epoll_wait 不会有这个问题),那么函数定义和函数调用的地方的 diff 具有相同的形式(比方说都是在倒数第二行加了一行内容),很容易肉眼验证有没有错位。如果参数写在一行里边,就得睁大眼睛数逗号了。
5. class 初始化列表的写法
同样的道理,class 初始化列表(initializer list)也遵循一行一个的原则,这样将来如果加入新的成员变量,那么两处(class 定义和 ctor 定义)的 diff 具有相同的形式,让错误无所遁形。以 muduo::net::Buffer 为例:
class Buffer : public muduo::copyable
{
public:
static const size_t kCheapPrepend = 8;
static const size_t kInitialSize = 1024;
Buffer()
: buffer_(kCheapPrepend + kInitialSize),
readerIndex_(kCheapPrepend),
writerIndex_(kCheapPrepend)
{
}
// 省略
private:
std::vector<char> buffer_;
size_t readerIndex_;
size_t writerIndex_;
static const char kCRLF[];
};
注意,初始化列表的顺序必须和数据成员声明的顺序相同。
6. 与 namespace 有关的缩进
Google 的 C++ 编程规范明确指出,namespace 不增加缩进。这么做非常有道理,方便 diff –p 把函数名显示在每个 diff chunk 的头上。
如果对函数实现做 diff,chunk name 是函数名,让人一眼就能看出改的是哪个函数。如下图,红色划线部分。
如果对 class 做 diff,那么 chunk name 就是 class name。
diff 原本是为 C 语言设计的,C 语言没有 namespace 缩进一说,所以它默认会找到“顶格写”的函数作为一个 diff chunk 的名字,如果函数名前面有空格,它就不认得了。muduo 的代码都遵循这一规则,例如:
namespace muduo { /// /// Time stamp in UTC, in microseconds resolution. /// /// This class is immutable. /// It's recommended to pass it by value, since it's passed in register on x64. /// class Timestamp : public muduo::copyable, public boost::less_than_comparable<Timestamp> { // class 从第一列开始写,不缩进
// 函数的实现也从第一列开始写,不缩进。
Timestamp Timestamp::now()
{
struct timeval tv;
gettimeofday(&tv, NULL);
int64_t seconds = tv.tv_sec;
return Timestamp(seconds * kMicroSecondsPerSecond + tv.tv_usec);
}
相反,boost 中的某些库的代码是按 namespace 来缩进的,这样的话看 diff 往往不知道改动的是哪个 class 的哪个成员函数。
这个或许可以通过设置 diff 取函数名的正则表达式来解决,但是如果我们写代码的时候就注意把函数“顶格写”,那么就不用去动 diff 的默认设置了。另外,正则表达式不能完全匹配函数名,因为函数名是上下文无关语法(context-free syntax),你没办法写一个正则语法去匹配上下文无关语法。我总能写出某种函数声明,让你的正则表达式失效(想想函数的返回类型,它可能是一个非常复杂的东西,更别说参数了)。更何况 C++ 的语法是上下文相关的,比如你猜 Foo<Bar> qux; 是个表达式还是变量定义?
7. public 与 private
我认为这是 C++ 语法的一个缺陷,如果我把一个成员函数从 public 区移到 private 区,那么从 diff 上看不出来我干了什么,例如:
diff --git a/muduo/net/TcpClient.h b/muduo/net/TcpClient.h --- a/muduo/net/TcpClient.h +++ b/muduo/net/TcpClient.h @@ -37,7 +37,6 @@ class TcpClient : boost::noncopyable void connect(); void disconnect(); - bool retry() const; void enableRetry() { retry_ = true; } /// Set connection callback. @@ -60,6 +59,7 @@ class TcpClient : boost::noncopyable void newConnection(int sockfd); /// Not thread safe, but in loop void removeConnection(const TcpConnectionPtr& conn); + bool retry() const; EventLoop* loop_; boost::scoped_ptr<Connector> connector_; // avoid revealing Connector
从上面的 diff 能看出我把 retry() 变成 private 了吗?对此我也没有好的解决办法,总不能每个函数前面都写上 public: 或 private: 吧?
对此 Java 和 C# 都做得比较好,它们把 public/private 等修饰符放到每个成员函数的定义中。这么做增加了信息的冗余度,让 diff 的结果更直观。
8. 头文件的排列顺序
除了必须放在首位的头文件,其余的都按字典序排列。这样将多人修改时冲突的可能性降到最小。另外,Makefile 中的文件列表也按字典序排列,以降低冲突的可能。
参考 muduo/**/*.cc
欢迎补充。
对 grep 友好的代码风格
操作符重载
C++工具匮乏,在一个项目里,要找到一个函数的定义或许不算太难(最多就是分析一下重载和模板特化),但是要找到一个函数的使用就难多了。不比 Java,在 Eclipse 里 Ctrl+Shift+G 就能找到所有的引用点。
假如我要做一个重构,想先找到代码里所有用到 muduo::timeDifference 的地方,判断一下工作是否可行,基本上惟一的办法是grep。用 grep 还不能排除同名的函数和注释里的内容。这也说明为什么要用 // 来引导注释,因为在 grep 的时候,一眼就能看出这行代码是在注释里的。
在我看来,operator overloading 应仅限于和 STL algorithm/container 配合时使用,比如 transform() 和 map<T,U>,其他情况都用具名函数为宜。原因之一是,我根本用 grep 找不到在哪儿用到了 operator-()。这也是 muduo::Timestamp 只提供 operator<() 而不提供 operator+() operator-() 的原因,我提供了两个函数 timeDifference 和 addTime 来实现所需的功能。
又比如,Google Protocol Buffers 的回调是 class Closure,它的接口用的是 virtual function Run() 而不是 virtual operator()()。
static_cast 与 C-style cast
为什么 C++ 要引入 static_cast 之类的转型操作符,原因之一就是像 (int*) pBuffer 这样的表达式基本上没办法用 grep 判断出它是个强制类型转换,写不出一个刚好只匹配类型转换的正则表达式。(again,语法是上下文无关的,无法用正则搞定。)
如果类型转换都用 *_cast,那只要 grep 一下我就能知道代码里哪儿用了 reinterpret_cast 转换,便于迅速地检查有没有用错。为了强调这一点,muduo 开启了编译选项 -Wold-style-cast 来帮助查找 C-style cast,这样在编译时就能帮我们找到问题。
一切为了效率
如果用图形化的文件比较工具,似乎能避免上面列举的问题。但无论是 web 还是客户端,无论是 inline diff 还是 diff by lines 都不能解决全部问题,效率也不一定更高。
对于(2),如果想知道是谁在什么时候增加的 double z,在分行写的情况下,用 git blame 或 svn blame 立刻就能找到始作俑者。如果写成一行,那就得把文件的 revisions 拿来一个个人工比较,因为这一行 double x = 0.0, y = 1.0, z = -1.0; 可能修改过多次,你得一个个看才知道什么时候加入了变量 z。这个 blame 的 case 也适用于 3、4、5。
比如(6)改动了一行代码,你还是要 scroll up 去找改的是哪个 function,人眼看的话还有“看走眼”的可能,又得再定睛观瞧。这一切都是浪费人的时间,使用更好的图形化工具并不能减少浪费,相反,我认为增加了浪费。
另外一个常见的工作场景,早上来到办公室,update 一下代码,然后扫一眼 diff output 看看别人昨天动了哪些文件,改了哪些代码,这就是一两条命令的事,几秒钟就能解决战斗。如果用图形化的工具,得一个个点开文件 diff 的链接或点开新 tab 来看文件的 side-by-side 比较(不这么做的话看不到足够多的上下文,跟看 diff output 无异),然后点击鼠标滚动页面去看别人到底改了什么。说实话我觉得这么做效率不比 diff 高。
(待续)