购买
下载掌阅APP,畅读海量书库
立即打开
畅读海量书库
扫码下载掌阅APP

1.2 坏代码的特点

“幸福的人都是相似的,不幸的人各有各的不幸。”这句来自经典名著《安娜·卡列尼娜》的名言也可以延伸到编码风格上——“良好的编码风格遵循统一的规范,坏代码各有不同的表现”。话虽如此,但根据笔者对大量代码的观察,坏代码的“坏”大致体现在如下4个方面。

● 排版:所有与代码的外观相关的问题都可以归于排版问题,比如不正确的对齐和缩进、省略该有的空格导致书写拥挤等。在良好的编码风格中,代码的对齐、缩进、空格都应该遵循统一的标准。

● 命名:包括不正确的术语、不符合习惯的名称、错误的时态、使用拼音、含有中文等特殊字符的文件名、命名风格不统一等。C语言本身虽然是独立的,但在传统上和英语息息相关,因此C代码中的命名也应该尽量遵循正确的英语语法和用法习惯。另外,变量和函数等的命名风格虽然没有“唯一正确”的标准,但在同一个程序或者项目中,命名风格应该统一。

● 过度使用:不分场合地过度使用某种语法特性。过度使用 typedef (类型定义)便是一个典型的例子。过度使用类型定义,会导致其他程序员在阅读代码时无法快速知悉一个类型到底是结构体、枚举量、指针还是整数。

● 注释:注释是一种重要的代码文档工具,可以帮助程序员阅读自己或他人的代码。但是,不规范的注释常常会影响我们对代码的阅读和理解,如注释太多或太少、注释风格不统一、注释太花哨、注释的内容和代码脱节等。

1.2.1 坏代码实例

具有上述这些问题的坏代码可以说是随处可见。程序清单1.1是一段C代码,它实现了一个简单的链表。

程序清单1.1 一个简单的链表实现(不良编码风格)
typedef struct Linklist{ ⇽---  1.应避免使用typedef类型定义。2.Linklist的命名不当。3.字符{前应该有空格。
    const char * elem;
    struct Linklist * next;
}linklist; ⇽---  4.字符}后应该有空格。5.linklist的命名不当。
 
//初始化链表的函数 ⇽--- 6.在C程序中,应避免使用C++风格的注释。7.应避免使用中文做注释。8.//后缺少必要的空格。
linklist * initlinklist(); ⇽--- 9.initlinklist的命名不当。10.函数声明不规范。11.表示指针的字符*应该紧贴后面的变量名或函数名。
 
const char * titles[]={"第1章 提高代码可读性","第2章 用好写好头文件",
"第3章 消除编译警告","第4章 常量的定义和使用",
"第5章 充分利用构建系统生成器"}; ⇽--- 12.书写拥挤,=两边应有空格。13.未适当换行和缩进。
 
int main() { ⇽--- 14.main函数的原型定义不规范。15.用于定义函数体的起始字符{应另起一行。
    // 使用章节标题初始化链表
    printf("初始化链表为:\n");
    linklist *p=initlinklist(); ⇽--- 16.赋值运算符=的两边应有空格。
    display(p); ⇽--- 17.未事先声明display()函数。18.display这一术语的选择不恰当,应考虑使用dump。
    return 0;
}
linklist * initlinklist(){ ⇽--- 19.函数体之间应有空行,以便于阅读。20.函数原型的定义不规范。21.表示函数返回值类型为指针的字符*应该紧贴后面的函数名。22.字符{的前面应该有空格。
    linklist * p=NULL;  //创建头指针 ⇽--- 23.表示变量类型为指针的*应该紧贴变量名。24.赋值运算符=的两边应有空格。
    linklist * temp = ( linklist*)malloc(sizeof(linklist)); ⇽--- 25.星号*和temp之间不应该有空格。26.左括号(和类型名称linklist之间有多余的空格。
 
    // 先初始化首元节点 ⇽--- 27.使用了不规范的术语“首元节点”。
    temp->elem = titles[0];
    temp->next = NULL;
    p = temp; // 头指针指向首元节点
 
    for (int i=1; i<5; i++) { ⇽--- 28.等号=和小于号<的两边应该有空格。
        linklist *a=(linklist*)malloc(sizeof(linklist));
        a->elem=titles[i];
        a->next=NULL;
        temp->next=a;
        temp=temp->next; ⇽--- 29.以上5行中等号=的两边都应该有空格。
}    30.未缩进,应和定义循环体的for语句对齐。
return p; ⇽--- 31.return语句之前应有空行,用于分隔不同的功能块。32.应缩进。
}

可以看到,在区区几十行的代码中,我们罗列了30多个问题(当然,大多数问题是重复的)。我们可以将这些问题归纳如下。

● 排版问题:排版问题主要表现在对齐、缩进、空格和空行的不恰当使用上。比如上述代码在该使用空格的地方没有使用空格,有些不该使用空格的地方却使用空格。这一方面使代码不够清晰和整洁,另一方面也表现出代码的作者在编码时非常随意。

● 命名问题:这段代码中存在明显的命名不当问题。代码中变量和函数的命名既要注意语法,也要注意命名风格。在一开始的 typedef 类型定义中, Linklist 这个名称同时存在这两个问题。链表的标准英语名称是linked list,在把这两个单词组合在一起构成类型名时,应该采用 LinkedList linked_list 这样的形式。类似地,变量名称 linklist 在风格和语法上都存在问题。再如 initlinklist() 这个函数,其命名也不规范,几个小写单词挤在一起既拥挤又难看。如果使用 init_linked_list() 作为函数名,用下画线将各个单词分隔,则会明显提高代码的可读性。

● 语法问题:这段代码存在两方面语法问题,一方面是不必要的 typedef 类型定义,另一方面是错误的 main 函数等的原型声明。比如这段代码中定义的链表结构体,并不需要使用 typedef 定义为一种新的数据类型,在代码中直接使用 struct linked_list 作为类型名称显然要比使用新定义的类型名称 linklist 更加清晰。原因在于通常我们会将类型定义放到头文件中,当我们在某个源代码文件中阅读到使用 struct linked_list * 的代码时,不需要查看头文件便可知悉该代码定义了一个结构体指针,而非整数、枚举量或者结构体。在本章的后面,我们将给出合理使用 typedef 的几个建议。另外,在这段代码中, int main() 的写法并不规范,规范的写法要么是 int main(void) ,要么是 int main(int argc, const char *args[])

● 注释问题:这段代码中的原始注释采用了C++风格的注释( // 打头),另外注释内容使用了中文。尽管强制非英语母语的程序员使用英文写注释有些刻板或者严苛,但如果我们考虑到开源大势以及可能的国际交流,尽量用英语书写注释无疑是一种合理的要求。笔者不鼓励在产品级代码中使用中文注释的另外一个原因是,大量的计算机软硬件术语最初源自英文,当我们使用中文时,由于理解或者表述上的问题,就会出现各种偏差。比如在上述代码中,“首元节点”这一术语就显得非常怪异。我们可以轻松理解“首”,这里大概就是指第一个;但“首元”或者“首元节点”是何意?另外,在C代码中,仅在简短的行尾注释中使用C++引入的注释方法,也是应该遵循的一个原则——这看起来有点古板,却是优秀和专业的C程序员始终需要遵循的一项传统或者习惯。

1.2.2 妙手理码

拿到坏代码时,一个很好的习惯就是对代码按照编码规范的要求进行整理。这一过程本身就是阅读代码的过程,通过整理,我们可以使代码变得更加清晰易懂,也可能发现并解决一些潜在的问题。

程序清单1.2是按照符合惯例的编码风格对程序清单1.1中的代码进行修改后的版本。

程序清单1.2 一个简单的链表实现(整理后)
#include <stdio.h>
#include <stdlib.h>
 
struct linked_list {
    const char         *title;
    struct linked_list *next;
}
 
/* Creates and initializes a new linked list. */
static struct linked_list *init_linked_list(void);
 
/* Dumps the contents of a linked list */
static void dump_linked_list(struct linked_list *list);
 
/* Destroys a linked list */
static void destroy_linked_list(struct linked_list *list);
 
static const char *titles[] =
{
    "第1章 提高代码可读性",
    "第2章 用好写好头文件",
    "第3章 消除编译警告",
    "第4章 常量的定义和使用",
    "第5章 充分利用构建系统生成器"
};
 
int main(void)
{
    /* Creates and initialize a new linked list with chapter titles */
    struct linked_list *list = init_linked_list();
 
    printf("A new linked list has been created and initialized: \n");
    dump_linked_list(list);     // dump the contents
 
    destory_linked_list(list);  // destroy the list
    return 0;
}
 
struct linked_list *init_linked_list(void)
{
    struct linked_list *head = NULL;
 
    /* allocates a node for the head of the linked list */
    struct linked_list *head = (struct linked_list*)malloc(sizeof(*head));
 
    /* initializes the head node */
    head->title = titles[0];
    head->next = NULL;
 
    struct linked_list *p = head;
    for (int i = 1; i < 5; i++) {
        struct linked_list *a;
        a = (struct linked_list*)malloc(sizeof(*a));
        a->title = titles[i];
        a->next = NULL;
 
        p->next = a;
        p = a;
    }
 
    return head;
}

可以看到,修改之后的代码排版错落有致,逻辑清晰,给人一种赏心悦目的感觉。除了排版,我们还在如下6个方面对原有的代码做了调整,以便提高代码可读性以及代码质量。

● 将 struct linked_list 结构体中的第一个成员( elem )更名为更具实际意义的 title

● 将 init_linked_list() 函数声明为 static 类型,避免命名污染。

● 将 display() 函数重命名为 dump_linked_list() ,并声明为 static 类型。 display 这个术语通常用于在窗口或者页面中显示一个图形,而在本例中,展示一个链表通常意味着将其内容转储(dump)到指定的文件或者标准输出(标准输出本质上也是文件),以便事后查看或者调试。因此,使用 dump 这个术语来命名这个函数,显然要比使用 display 强很多。

● 新增 destroy_linked_list() 函数,用于销毁新创建的链表,并声明为 static 类型。原有代码在 main() 函数返回时并未做内存的清理工作。尽管在这个简单的链表实现中不必如此严谨,但作为专业程序员,我们应该养成良好的习惯并逐渐形成条件反射:既然有链表的创建函数,就应该有对应的链表销毁函数。

● 在 init_linked_list() 函数的实现中,移除了不必要且易混淆的 temp 变量。

● 使用 sizeof(*head) 的写法替代了 sizeof(struct linked_list) 的写法,避免代码行过长。

注意,上述代码中未包含 dump_linked_list() destroy_linked_list() 两个函数的实现,有兴趣的读者可自行实现。 Uz2hgmps/UxIjTi0G3mcPzJGPHBdq049bu7HNOHCle+nQe9uHMWJ3fwDqj59WDS+

点击中间区域
呼出菜单
上一章
目录
下一章
×