编写高质量代码

来源： yebangyu （@睡眼惺忪的小叶先森 ）

链接：www.yebangyu.org/blog/2015/11/06/how-to-write-elegant-cpp-codes/ （点击尾部阅读原文前往）

已获转载授权

我们知道，int 和 double 能表示的数值的范围不同。其中，64 位有符号整数的范围是 [-9223372036854775808,9223372036854775807]，而 64 位无符号整数的范围是 [0,18446744073709551615]。这两个区间有一定的 overlap，而 double 可以表示的范围更大。

现在，需要编写两个函数: 给定一个 double 型的 value，判断这个 value 是否是一个合法的 int64_t 或者 uint64_t。本文说的 “合法”，是指数值上落在了范围内。

bool is_valid_uint64 ( const Double &value );

bool is_valid_int64 ( const Double &value );

这里我们用 Double 而不是 double，原因是我们的 double 不是基础数据类型，而是通过一定方法实现的 ADT，这个 ADT 的成员函数有：

class Double

{

public :

int get_next_digit ( bool &is_decimal );

bool is_zero ();

bool is_neg ();

};

通过调用get_next_digit，可以返回一个数字，不断调用它，可以得到所有 digits。举个例子，对于值为 45.67 的一个 Double 对象，调用它的get_next_digit成员函数将依次得到

4 is_decimal = false // 表示整数部分

5 is_decimal = false // 表示整数部分

6 is_decimal = true // 表示小数部分

7 is_decimal = true // 表示小数部分

当get_next_digit返回 -1 时，表示读取完毕。

如何利用 Double 类里的成员函数，来实现is_valid_uint64和is_valid_int64这两个函数呢？

一些新手可能会写这样的代码：

bool is_valid_uint64 ( const Double &value )

{

bool is_valid = true ;

int digits [ 2000 ];

int counts = 0 ;

if ( value . is_zero ()) {

is_valid = true ;

} else if ( value . is_neg ()) {

is_valid = false ;

} else {

bool is_decimal = false ;

int digit = 0 ;

while (( digit = value . get_next_digit ( is_decimal )) != - 1 ) {

if ( is_decimal ) {

is_valid = false ;

break ;

} else {

digits [ counts ++ ] = digit ;

}

}

uint64_t tmp = 0 ;

uint64_t base = 1 ;

for ( int i = counts - 1 ; i >= 0 ; i ++ ) {

tmp += digits [ i ] * base ;

if ( tmp > UINT64_MAX ) {

is_valid = false ;

break ;

}

base *= 10 ;

}

}

return is_valid ;

}

bool is_valid_int64 ( const Double &value )

{

bool is_valid = true ;

int digits [ 2000 ];

int counts = 0 ;

if ( value . is_zero ()) {

is_valid = true ;

} else if ( value . is_neg ()) {

bool is_decimal = false ;

int digit = 0 ;

while (( digit = value . get_next_digit ( is_decimal )) != - 1 ) {

if ( is_decimal ) {

is_valid = false ;

break ;

} else {

digits [ counts ++ ] = digit ;

}

}

uint64_t tmp = 0 ;

uint64_t base = 1 ;

for ( int i = counts - 1 ; i >= 0 ; i ++ ) {

tmp += digits [ i ] * base ;

tmp *= - 1 ;

if ( tmp INT64_MIN ) {

is_valid = false ;

break ;

}

base *= 10 ;

}

} else {

bool is_decimal = false ;

int digit = 0 ;

while (( digit = value . get_next_digit ( is_decimal )) != - 1 ) {

if ( is_decimal ) {

is_valid = false ;

break ;

} else {

digits [ counts ++ ] = digit ;

}

}

uint64_t tmp = 0 ;

uint64_t base = 1 ;

for ( int i = counts - 1 ; i >= 0 ; i ++ ) {

tmp += digits [ i ] * base ;

if ( tmp > INT64_MAX ) {

is_valid = false ;

break ;

}

base *= 10 ;

}

}

return is_valid ;

}

这样的代码，存在诸多问题。

设计问题

不难发现，两个函数存在很多相似甚至相同的代码；而同一个函数内部，也有不少代码重复。重复的东西往往不是好的。重构？

性能问题

先获得所有 digits，然后从最低位开始向最高位构造值，效率较低。难道没有可以从最高位开始，边获得边计算，不需要临时数组存储所有 digits 的方法吗？

正确性问题

随便举几个例子：

第 24 行，tmp += digits[i] * base; 有没有考虑到可能的溢出呢？

第 68 行，难道有小数部分就一定不是合法的 int64 吗？那么，123.000？嗯？

规范问题

帅哥，这么多代码，一行注释都没有，这样真的好吗？

因此，毫无疑问，这是烂代码，不合格的代码，需要重写的代码。

以下是我个人认为比较好的设计和实现，仅供参考。

bool is_valid_uint64 ( const Double &value )

{

bool ret = false ;

check_range ( value , &ret , NULL );

return ret ;

}

bool is_valid_int64 ( const Double &value )

{

bool ret = false ;

check_range ( value , NULL , &ret );

return ret ;

}

void check_range ( const Double &value ,

bool * is_valid_uint64 ,

bool * is_valid_int64 ) const

{

/*

* 对于一个负数的 value，它不可能是一个合法的 uint64.

* 因此，只剩下三种可能：

* I 输入的 value 是负数，判断是否是合法的 int64

* II 输入的 value 是正数，判断是否是合法的 uint64

* III 输入的 value 是正数，判断是否是合法的 int64

* 对于第 II、III 这两种情况：只要判断 value 的值是否超过 uint64、int64 的上界即可

* 对于第 I 种情况，我们利用 - A > -B 等价于 A

* 因此，在第 I 种情况里，可以判断 value 的绝对值，是否超过 int64 的最小值的绝对值即可。

* （int64 的最小值的绝对值？那不就是 int64 的最大值？哦，不！）

* 因此，不管哪种情况，判断绝对值是否超过某个上界即可。

* 这三种情况，上界不一样。把三个上界存到了一个二维数组 THRESHOLD 里

*/

bool * is_valid = NULL ;

static const int FLAG_INT64 = 0 ;

static const int FLAG_UINT64 = 1 ;

static const int SIGN_NEG = 0 ;

static const int SIGN_POS = 1 ;

int flag = FLAG_INT64 ;

if ( NULL != is_valid_uint64 ) {

is_valid = is_valid_uint64 ;

flag = FLAG_UINT64 ;

} else {

is_valid = is_valid_int64 ;

}

* is_valid = true ;

if ( value . is_zero ()) {

//do nothing。0 是合法的 uint64，也是合法的 int64

} else {

int sign = value . is_neg () ? SIGN_NEG : SIGN_POS ;

if (( SIGN_NEG == sign ) && ( FLAG_UINT64 == flag )) {

* is_valid = false ; // 负数不可能是合法的 uint64

} else {

uint64_t valueUint = 0 ;

static uint64_t ABS_INT64_MIN = 9223372036854775808ULL ;

//int64        uint64

static uint64_t THRESHOLD [ 2 ][ 2 ] = { { ABS_INT64_MIN , 0 }, //neg

{ INT64_MAX , UINT64_MAX } }; //pos

int digit = 0 ;

bool is_decimal = false ;

while (( digit = value . get_next_digit ( is_decimal )) != - 1 ) {

if ( ! is_decimal ) {

// 为了防止溢出，我们不能这么写:

//"value * 10 + digit > THRESHOLD[index]"

if ( valueUint > ( THRESHOLD [ sign ][ flag ] - digit ) / 10 ) {

* is_valid = false ;

break ;

} else {

valueUint = valueUint * 10 + digit ; // 霍纳法则（也叫秦九韶算法）

}

} else {

if ( ! digit ) {

* is_valid = false ; // 小数部分必须是 0；否则不可能是合法的 uint64、int64

break ;

}

}

}

}

}

}

代码规范

团队的代码规范，一般由领导和大佬们制定后，大家统一实行。这里面有几个问题：

真的需要代码规范吗？

言下之意，制定和执行代码规范是否浪费时间？

答案是：It depends。如果项目很庞大、代码质量要求很高，那么，制定和执行代码规范所花费的时间，将大大少于后期因为不规范开发带来的种种调试和维护成本。如果是小打小闹的代码，就无所谓了。

代码规范的制定为什么这么难？

原因众多，其中一个很重要的部分是团队每个人的口味和观点不尽相同。就代码风格而言，有人喜欢对内置类型变量 i 使用 i++，有人坚持认为应该使用 ++i 不管 i 是不是复杂类型。因此，制定代码规范需要在讨论之后最后拍板决定，这里面甚至需要独裁！是的，独裁！

代码规范制定需要注意什么事项？

如果代码规范限制太松，那么等于没有规范；如果太严，大大影响开发效率。这里面的尺度，需要根据项目需要、团队成员特点全面考量，进行取舍。

需要注意的是，没有任何一种代码规范是完美的。例如，在 C++ 中，如果启用异常，那么代码的流程将会被各种异常处理中断，各种 try catch throw 让代码很不美观；如果禁用异常，也就是在开发的过程中不能使用异常特性，那么团队成员可能因为长期没有接触这项语言 feature 而造成知识和技能短板。

代码风格举例

举两个我认为比较重要、比较新鲜、比较有趣的代码风格。

1、使用引用需要判空吗？

void f ( int &p );

void g ( int * p );

我们都知道，在 g 中，使用 * p 前需要对 p 是否为 NULL 进行判断，那么 f 呢？如果质量非常关键、代码安全非常重要的场景，那么实际上，也是需要的。因为调用者可能这样：

int * q = NULL ;

//......

f ( * q );

因此，需要在 f 里增加if(NULL == &p)的判断。

2、级联 if else 语句。

首先看一个我个人认为不好的代码风格：

int f ( int a , int b )

{

if ( a >= 1 ) {

if ( b >= 1 ) {

if ( a >= b ) {

//do sth

} else {

//error1

}

} else {

//error2

}

} else {

//error3

}

}

这个函数的核心在于 do sth 部分。其实我们可以改写为级联 if－else 形式，如下：

int f ( int a , int b )

{

if ( a 1 ) {

//error3

} else if ( b 1 ) {

//error2

} else if ( a b ) {

//error1

} else {

//so, a>=1 && b>=1 && a>=b

//do sth

}

}

是不是优美多了？前面只做一些错误处理、前期准备、参数检查等，最后的 else 分支做实实在在的功能性事情。

Code Review

什么是 Code Review？

很多人把它翻译为代码审查，我觉得太政治味了。程序员尤其是新手写完代码后，可能会有风格问题（比如不符合团队的代码规范）、安全性问题 (比如忘记指针判空)、优雅性问题 (比如大量冗余代码)、正确性问题（比如算法设计错误），那么在发布代码到公共库之前，提交给师兄或者 mentor，让他帮你 review 一下代码，并提出可能的问题和建议，让你好好修改。这样的过程，就叫做 Code Review。

我的天呐，那这不是很占用时间？

是的。一个写代码，一个看代码，看代码的时间可能并不比全新写一份代码少。那么，这又是何必呢？

主要的原因有：

1、review 确实占用了开发时间，然而开发，或者说写代码，其实只占很少的时间比例。很多时间花在 debug、调试、写文档、需求分析、设计算法、维护等等上。

2、代码质量非常重要，这点时间投入是值得的。与其后期苦逼追 bug，不如前期多投入点时间和人力。

3、培养新人，让新手更快成长。

如何更好的执行 Code Review

这里给几点建议：

1、不走过场。走过场，还不如不要这个流程。

2、作为 Reviewer，看不懂代码就把作者拉过来，当面询问，不要不懂装懂，也不要爱面子不好意思问。

3、作为 Coder，心里要有感激之情。真的。不要得了便宜还卖乖，感恩 reviewer，感激 reviewer 对自己的进步和成长所做出的贡献，所花费的心血。中国人里狼心狗肺、忘恩负义、不懂感恩的人还算少吗？

4、作为 Coder，给 Reviewer Review 之前，请先做单元测试并确保通过，并自己尝试先整体看一遍自己本次提交的代码。注意，不要给别人提还没调试通过的代码，这是非常不尊重别人的表现。

质量保证

1、测试不是专属 QA 的活儿，对自己写的代码提供质量保证，是程序员的职责。QA 要负责的，是系统的质量，不是模块的质量。

2、测试，需要意识，需要坚持。我发现 C++ 程序员、前端程序员的测试意识或者说质量意识最强；数据科学家或者数据工程师的质量意识最差，很多人甚至不写测试用例。当然，这不怪他们，毕竟，有时候代码里有个 bug，准确率和召回率会更高。

3、测试用例的编写和设计需要保证一定的代码覆盖率，力求让每个分支和流程的代码都走到，然后分析运行结果是否是符合期望的，不要只考虑正确路径上的那些分支。

4，测试用例的编写和设计力求全面，考虑到方方面面。以非常经典的二分搜索为例：

int binary_search(int *p, int n, int target, int &idx);

binary_search 函数返回值为 0 表示成功执行，输出参数 idx 返回 target 在有序数组 p 中 (第一次出现) 的位置，－1 表示不存在。

那么测试用例至少应该涵盖：

• p 为 NULL 的情况

• 数组大小 n 分别为负数、0、1、2 时情况

• 数组 p 不是有序数组的情况

• target 在数组中出现 0 次、1 次、n 次的情况

你是否都考虑到了呢？

4、有时候，自己书写测试用例显得刀耕火种，现在已经有很多辅助的工具，读者可以自行 google 一下。

---

● 本文编号1946，以后想阅读这篇文章直接输入 1946 即可。

●本文分类“ C ”、“ 项目管理 ” ， 搜索分类名可以获得相关文章。

●输入m可以获取到文章目录

本文内容的相关公众号推荐

Linux学习 ↓ ↓ ↓

C/C++编程 ↓ ↓ ↓

更多推荐 《 15个技术类公众微信 》

涵盖：程序人生、算法与数据结构、黑客技术与网络安全、大数据技术、前端开发、Java、Python、Web开发、安卓开发、iOS开发、C/C++、.NET、Linux、数据库、运维等。