常跟人说,今年是我编程第十年了,听起来很夸张,不过是真的。经过2010、15和17年,现在算有自己的编码习惯和风格;但是每次回头看前一阵的代码,都会有很多地方不满意,甚至确信不该那么写;或许是因为懈怠,或许是并未熟记本应坚持的好习惯。
360离职前交接代码,老大帮我把1、2、3、4、5、6、7改成enum结构体,用XXXX_BIT代替;甚为惭愧,于是把一坨if-else改成switch-case,具体内容抽象成函数。写go parser那段时间,扶摇教给我如何写规范的git commit log,以及scala如何优雅的关闭文件描述符。这些细节让我印象深刻,好看的代码是如此重要,列一些点提醒自己时刻牢记。
函数里尽量不要出现数字和字符串常量
最低级的代码就是充斥着大量数字如0、1、-1等,没人知道是什么意思,作为函数返回值,意义极其不明确;或者各种裸字符串"something wrong",每次都要跳到代码逻辑里修改。正确的做法是定义成宏或者const常量,或者enum(c++11以后考虑enum class)。
不管是用作返回值,还是标志位、大小等,都会变得意义明确;而且放在合适的头文件,一次修改,全体收益。
我要是review代码,绝对不给出现数字和字符串的通过。
#define INVALID_FD -1
#define STATUS_OK 0
#define STATUS_FAIL 0
#define TEXT_FAIL "something wrong"
if (get_file_content(data)==STATUS_OK) {
...
}
命名风格统一且清晰明了
最好能通过名字,一眼看出是类、函数、const函数还是宏,是全局变量、静态变量、局部变量还是常量;是锁、文件描述符、socket还是指针;功能是get、set、push、pop还是del。类的成员变量和成员函数,也要保持一致的明明习惯,能通过名字看出是类成员。
其实参考内核代码就很好,例如Windows一般是大驼峰,而linux则是小写加下划线。而c++类名一般大写开头,宏一般全大写,而函数则全部小写加下划线;变量名加单字符前缀用以区分。
// 例如全局变量加g_, 静态变量加c_,类成员变量加_,局部变量什么都不加
g_global_num, s_static_num, c_const_num, m_local_num, _class_data
g_mutex, g_log_fd, g_server, client, sock, ret
控制好空格和回车
if(x>0){...}
for(int i=0;i<100;++i){
...
}
int x(){
...
}
int y(){
...
}
上面这段代码看着很乱,好看的应该是这样的:
if (x>0) {
...
}
for (int i=0; i<100; ++i) {
...
}
int x() {
...
}
int y() {
...
}
声明放在合适的头文件/源文件
尽量把公用的声明放到头文件,例如宏、通用函数,不仅能减少代码,还会促使人们更好的组织自己的代码结构。举个例子,redis代码风格很差,而nginx和leveldb简直是典范。
这还涉及到文件划分,什么函数或类应该单独抽出来,哪些可以混在一起;还是要根据耦合程度,完全独立且可能被其他地方引用的代码,就要公用。我见过每个文件都有一个类似convert_to_string的函数,互相拷贝,连抽成一个common.cpp都懒得做。
头文件放太多东西,也会导致编译速度下降,很多人提倡PImpl,可惜这个要求太高了,最后难免弄得更乱。
但是有一点要牢记:项目里不出现两个功能一模一样的函数。
引用的每个头文件都要知道其作用,且规范引用路径
这个简直是编译灾难,而且新人看代码会一脸懵逼,想单独写个简单测试程序,甚至得把所有项目依赖搞过来。
要我说,就不应该有把所有头文件include到一起,功能划分好,按需引用;而且引用路径要规范,有的是#include "test.h",有的是#include 。
最好的结果是把源文件、引用的头文件拎出来,能直接编译;不然就是不规范。手游拍卖平台拔出萝卜带出泥,光引用的头文件就十几个,它们又引用一堆,代码怎么能算组织好。
函数参数尽量不要超过四个
经常见到这样的函数:
int set_something_to_redis(int count, int number, DataInfo* info, int timeout, bool exist, int& succ_num, bool& result, int& used_time) {
...
}
真的太长了,要么把参数组织成结构体:
#define __OUT
enum class REDIS_OPT {
REDIS_GET=0,
REDIS_SET=1,
...
};
struct RedisOptData{
REDIS_OPT opt;
DataInfo *info;
__OUT int succ_num;
__OUT bool result;
};
要么回车分割好,堆在一行真的不行。
控制函数行数,小屏幕也能看全
一个好的函数应该是这样的(每行也尽量注意,别有的太长,其他太短):
int good_function(Param* param)
{
int status=STATUS_OK;
do {
if (!check_param(param)) {
ERR_LOG(TEXT_PARAM_ERROR);
status=STATUS_PARAM_ERROR;
break;
}
if (do_thing1(param) !=STATUS_OK) {
...
break;
}
...
} while (0);
release_resources();
return status;
}
整个屏幕能看到所有代码,如果已经足够抽象,还是太长,那就没办法,业务太复杂。
尽量让每条语句逻辑独立且意义完整
也是针对函数,从头看到尾,每句话都要跟这个函数相关,而不要把人带出去思考:这个是干啥的?怎么实现的不重要,做了什么一定要清晰,要么函数命名规范,要么加注释。
更进一步,需要多条语句完成的工作,为什么不抽象成另一个函数?如果觉得函数调用影响效率,inline或者lambda或者宏?
这里有一条边界,一个工作需要多个工作流程来完成,每个流程是一条语句(包括函数调用、for循环等语句块);如果一个流程需要不止一个操作,那么这个流程就是可以抽象成单独的函数。
这其实也对函数做了一定要求:函数名字即所功能实现,函数功能要么很新鲜很简短。大家应该都见过好多个代码相似难辨差异的函数,绝大多数流程一样,只是为了业务需求,拷贝过来做了简单修改,却留下一堆垃圾。
有一次,某个并不复杂的业务需求,同事写了个递归全排列(先不说代码的bug:错误检查、死循环等),这个风格并不好。如果需要全排列,我们可以写个生成全排列的函数,得到排列结果再进一步处理;而不是直接把业务逻辑揉在一起。
局部变量在最小作用域声明
经常见到函数开头几句定义一大堆变量,实际上很多只在某个for循环内部用到,这个完全可以定义在for里头。我的意见是尽量靠近第一次使用的地方定义,同时尽量限定在最小作用域;还有一个好处,在循环这种场景,每次都会执行默认初始化,不