初级程序员常犯错误都在这里啦！

源 / 顶级程序员        文/ 曹春晖

最近接手其他人做的项目，导致之前的一些幻想破灭了。因为刚工作的时候做项目是php，而php本身的web框架一般只简单区分mvc，稍微麻烦一些的会多个library或者helper之类的。这样分层很少有优点同时也有缺点。当然了，现代的框架一般支持namespace，你也完全可以借鉴其它语言来做自己的内部框架。这里先不说这个。

mvc的优点自然是简单，无论一个新人有没有做过相关的工作，你只要跟他简单说明每一层的职责是什么，马上就可以开始工作。缺点也非常明显，因为太简单，所以代码在累积到一定量以后会变得难以控制复杂度。同时容易让没什么经验的程序员把代码写得难以维护。

所以之前听说java的框架这方面做得好一些，然后对部门的java项目抱有一些幻想。不过在实际接手后，发现理想和现实真是有比较巨大的鸿沟。所以先来总结一下共通的初级程序员比较容易犯的错误吧。如果哪天自己带团队了，面试别人也可以拿这些题作为区分人的一种界限。做项目的时候有思考的人和不思考的人还是会有不小的区别的。

1、命名太啰嗦，或者不规范

这个问题其实挺普遍的，即使是科班出身的人，工作了很多年的人，写程序也可能冒出来一堆var a/b/c之类的变量。如果你的程序本身短小精悍，或者只是纯粹的算法题，这样似乎问题不大(反正oj ac了以后你可能也就再也不会看自己的代码。但这样的代码放在生产环境中，问题就很大了。首先有些变量本身的生命周期可能会超过一个横着放的屏幕。这里逗逼一下，不是谁都能买得起Dell的可横可竖的屏幕好吗。比如按照我的13寸笔记本来看，vim的全屏模式下只能显示33行(当然了，我视力不好，字体比较大)。所以如果在第一行声明了一个变量var a。那么顺着读下去，在33行之外。。。我觉得我肯定不记得这是什么东西了。

如果你想要声明一个变量，一定要让这个变量的名字本身具有含义，比如productName，productPrice，userId，userInfo，orderHistory。但保证名字有含义的同时，一定要保证名字和其含义对应得上。并且不要把一个命名好的变量以优化的名义反复存储各种不同的值。例如我们这里的代码就曾经出现过看起来是userIds，但实际上打印出来却是一堆userInfo的信息的事情。着实让人恼火。

业务内部逻辑的变量命名还是讲究单词打全，意义明了等等。如果是框架本身的一些内容命名的话。例如java的spring框架里的service命名，反而应该稍微简洁一些。例如：

KnowledgeService kService;
OrderService oService;
UserMapper uMapper;

因为在整个class内部，service、mapper一般只会有唯一的对象，所以这里就算看不明白了去整个类的头部也能找得到。因为这些对象使用频率很高，所以简短一些对打字有好处(其实你是想偷懒吧)。当然了，如果你喜欢把名字打全，那我觉得也没什么不可。

变量命名之外，函数命名也很重要

例如上一家公司就有人会把函数叫dealData或者handleData，怎么看怎么别扭。。换成formatData是不是就很明了了？细节之处见真章。

2、魔法数字

这个问题在哪里都看得到，最简单的例如各种订单的status跳转。你会发现各种updateStatus(1)之类的神奇代码。如果恰巧数据库的表定义里又没有这个status的定义，那必然会变成维护人员的噩梦。解决方法很简单，在配置文件中集中维护这些特殊变量。

再进一步的话，应该引入各种方便的配置系统。例如java里常用的disconf，可以在不对系统上下线的情况下在配置系统里看到配置的key和value，并且可以即时地进行修改和配置下发。如果是其它语言没有类似系统的话，也可以借助现在流行的etcd或者zookeeper来自己开发，不会太复杂。

如果你发现自己的系统里充斥着各种不明所以的数值的话，那么就是时候考虑引入配置进行管理了。(其实从一开始就应该做这些事情)

3、解决了魔法数字的问题，但不对配置文件进行分类

有了配置系统以后，其实还有个麻烦的问题。就是所有配置都放在一个文件里。例如之前做的系统，把所有key和对应的sql都放在一起，然后导致配置文件变得很大很难看。想改个简单的sql连找都找不到在哪里。

改进方法很简单，第一是对配置文件进行业务逻辑分类，例如Order相关的配置项就放在Order的下面。如果配置内容还是很多，可以进一步进行粒度的细分，例如OrderHistory, OrderType之类的。其实一般的系统也不会这么复杂。

4、分类粒度问题

对配置进行了分类，但有时候也有可能配置粒度实在太细，导致文件很多，找起来还是很不方便。

这怎么办呢？

第一是粒度划分要掌握好度，第二是相关的配置最好有对应的comment可以拿来进行简单的检索。第三么，那可能就是将系统的模块进行拆分了。不同模块管理自己的配置文件。眼不见心不烦。

划分之后的模块是使用json http还是用rpc通信，看自己的业务量和延迟要求来定吧。

5、函数写的太长

这也是常见的问题。业务开发在堆代码的阶段特别容易引起这种问题。例如上一家公司的createOrder逻辑，一开始只是80行的小函数。但后来陆续加入了商品内容校验，用户对商品的合法性校验(主要是指用户是否是跨越了购买区域啊，或者购买了本没有资格购买的vip商品)，价格校验(是否优惠，优惠了多少，前端的价格有没有问题之类的)等等。然后就让事情变得越来越难以收拾，80行的开头最终变成了1000行的巨无霸。

这个又怎么解决呢？

首先是要根据功能进行简单的小函数块划分。

func ABCDE() {
  A
  B
  C
  D
  E
}

=>

func main() {
  A()
  B()
  C()
  D()
  E()
}

再具体一些

func createOrder(userInfo UserInfo, products []Product) {
  checkUserValid(userInfo);
  checkProducts(products);
  checkUserAndProducts(userInfo, products);
  var order = insertOrder(userInfo, products);
  var createFlag = createOrderDetail(order, products);
  return createFlag;
}

看起来就清晰多了，如果本身有上千行，进行划分之后每个函数也不会太长。

当然了，也有人会说，我的api逻辑怎么可能这么简单。有很多其它系统依赖于我当前系统的数据，我创建了订单以后要对他们的接口进行回调啊。这部分代码怎么都节省不下来吧？

其实还是可以的。

这一“点”其实就是消息队列的存在意义了。在后面的滥用回调一节中会做详述。

6、滥用回调，增加系统复杂性

滥用回调其实挺常见的，特别是在现在这个公司。很多时候有数据依赖的项目会彼此之间搞一堆错综复杂的回调接口。这样在初期开发的时候因为调用是同步操作，所以看起来项目很简单，而且同步调用随时打日志，出了问题也容易排查。

但依赖项多了以后就会变成维护和重构的噩梦。

举个实际场景例子：

这里有一个用户的评论系统，评论系统会对服务你的商家进行一些tag勾选和内容填空。

对于评论系统本身，只需要简单记录被打tag和被评论的对象到mysql即可。

但后面有信用系统、用户画像系统、客服系统依赖于这些评论的数据，所以需要把这些评论tag和内容同步给其它的几个系统，或者甚至是跨部门的系统。

目前是怎么做的呢？通过回调。

回调逻辑在一个接口里超过五个之后，程序员就不知道自己的代码是干什么的了。。在现在互联网公司每年离职率这么高的情况下还会导致pm和rd都离职了以后，后来的新人根本就不敢碰这些回调的问题。本来很简单的接口，50行实现完成了自己的业务逻辑，但是几百行都是在回调别人的系统。着实恶心。

这个问题想要解决起来也不难，其实根本就不是该解决，这种问题在一开始就应该极力避免。可以使用消息队列来避免这个问题。我们可以回想一下人们常说的消息队列可以用来解耦。所谓解耦，其实指的就是上面这种场景。在你的系统里发生了一个事件，其它系统对这个事件的数据有依赖，那么就让他去订阅你的系统里产生的消息，这条消息只要放在队列里即可。你喜欢用kafka还是其它的消息队列其实都是可以的。

这样，变化是下面这样的：

event A happend
then {
    call sys A1();
    call sys A2();
    call sys A3();
    call sys A4();
    call sys A5();
    call sys A6();
    call sys A7();
    call sys A8();
    ...
}

=>

event A happend
then {
    push msg to msg queue
}

A1~AN subscribe topic A in msg queue

复杂性被分散去了各自的系统中。看起来是不是明朗了很多。

当然，这种方案也不是没有问题。一旦引入消息队列，那么整个系统对消息队列本身的可靠性就有一定的考验。其次，原来的同步回调变成了基于消息队列的异步分布式系统。说到这种系统，大多数人肯定会想到让人头痛的分布式事务。而一般分布式事务还就只是数据库领域说的比较多，涉及到消息队列的分布式事务相对则会再复杂一些。所幸的是现在也有一些解决这种问题的思路，虽然资料不多，感兴趣的读者可以搜索一下saga pattern。实际上即使是saga pattern也没有完美解决这种问题。

实际项目里出错的概率其实挺低的，如果push失败打失败日志报警，并及时解决的话其实大多数的系统也没有这么大的成本。只要在失败时准备好恢复的预案即可。例如一些两边系统进行时间区间内数据同步的工具。要保证一定程度的幂等。

7、虽然有分层，但每层的结果返回没有明确界线和定义

这个在啥语言的项目里都比较多。

比如最近接触的java项目，分了entity/mapper/provider/service/controller几层。

其实是很科学的分层，但实际上在service这一层的使用上因为缺乏使用约束和规范，乱用的结果就是返回结果很混乱。例如有些函数是简单地返回了查询到的对象/对象数组，但另一些可能是直接把最终呈现给用户的business status code都进行了返回。

这样的话后来的维护者在接到需求的时候，难以根据直觉判断组合哪些小函数就能实现新功能。只能一个一个地去看你的具体函数实现。

这里似乎又体现出了设计模式和规范作为程序员共同语言的重要性，哈哈。

8、明明提供的是公共接口，但是其本身却只能使用一次

这个错误。。其实很简单，就是常说的可重入不可重入的概念，当年刚毕业的时候我也被很多人问到，但那个时候确实没法理解。不过其实很简单，如果你的函数在被调用时不能保证返回结果的正确性，例如使用了全局变量且没有加锁，或者使用了静态局部变量，那么这个函数就是不可重入的。但如果你的环境是php的话，这里又有一些微妙的不同。因为php本身是单线程运行，所以所谓的不可重入就只是你丫别用全局变量来存储函数计算的结果啊。。。

换到其它语言的话，其实就是尽量还是写可重入的函数，即使性能上稍微有些问题，但对于大多数的Web程序而言，你所谓的性能问题都是扯jb蛋。

这里为了不扯jb蛋，我们举个例子：

在这个部门的XX系统里有这么一段php代码：
global $validUserList = array();

public static getValidUserList($users) {
  global $validUserList;
  for($users as $user) {
    $validUserList[] = $user;
  }
}

代码的作者很自以为聪明地使用了一个全局的validUserList的变量。然后在getValidUserList里对该global变量进行修改。

但是关键问题是。。没有在每次调用的时候进行初始化，导致了这个函数根本就只能被调用一次的问题。如果先后调用多次，那么一定会得到匪夷所思的结果。

更关键的问题是，这样的代码竟然出现在线上运行的业务系统里(你们真的不是一个外包团队吗

这种问题改起来很简单，犯错的人只能说也比较弱智了。。

public static getValidUserList($users) {
  $validUserList = array();
  for($users as $user) {
    $validUserList[] = $user;
  }
  return $validUserList;
}

不要觉得每次都声明新的数组会导致性能问题。你的web程序不在乎那一点性能。

9、设计模式滥用

这一点可能是不太好界定的一点，用设计模式本身就是为了把重复的工作进行一次性化。但问题很多时候不在于你用了什么设计模式，而在于写这段代码的人是谁。比如有人用的明明是策略模式，但你在他的代码的字里行间都看不出来这是策略模式，只看到什么getOm(其实是莫名其妙的getObjectModel的缩写)，再通过反射去找到一个写死了名字的xxxxfunction，再直接通过字符串进行函数调用(php才可以这样)，读得人云里雾里。而且待你把他的代码全部扫过一遍之后才发现，虽然用了策略模式，但这段代码只有一种策略，其功能只是把数据库里的一个表的一个字段修改为一个固定的状态值。

实际上只需要三行代码就可以解决的问题。

实际上这位程序员写了多少代码呢？

1000行。。。

你是用代码量来衡量工作量的公司的员工吗？

10、访问数据库不做批量

比较典型的场景，现在大多数的web程序都可以分为列表页和详情页。。。说批量，其实主要说的就是列表页的问题。

例如一个商品列表页，里面有五十种商品，每个商品有一个分类，在商品表里只存储了分类的id，而在展示的时候需要把分类的路径和分类的名字都展示出来。

func getProductList(xxxx) {
   var products []Product
   for product := range products {
      categoryName := getCategoryName(product.getCategoryId())
      product.setCategoryName()
   }
}

这个问题看起来似乎问题不大？问题大了去了。你想想本来查询两次就可以解决的问题，50种商品，你就需要至少查询51次数据库。像php/py这样的语言，如果没有数据库连接池，那每次查询都需要去和数据库建立连接，这个问题就更加的严重，本来几毫秒的接口变成了几秒或者几十秒。

即使是有连接池可以复用连接的语言，你查询50次和2次的性能会一样么？

11、树形结构的表结构设计问题

公司里有人会把存储树形结构的表设计成这样：

 Field            | Type         | Null | Key | Default             | Extra                       |
+------------------+--------------+------+-----+---------------------+-----------------------------+
| id               | int(10)      | NO   | PRI | NULL                | auto_increment              |
| name             | varchar(64)  | NO   |     |                     |                             |
| parent_id        | int(10)      | NO   |     | 0                   |                             |
| status           | tinyint(3)   | NO   |     | 0                   |                             |
| createtime       | datetime     | NO   |     | 0000-00-00 00:00:00 |                             |
| modifytime       | timestamp    | NO   |     | CURRENT_TIMESTAMP   | on update CURRENT_TIMESTAMP |
| category_id_path | varchar(256) | NO   |     | 0                   |                             |

先看表结构，有没有什么问题？对，没有存储level信息，这种情况下如果我要找三/四级分类的所有分类id，那么就变成了一件非常麻烦恶心不可能的事情。何必呢。

除了level之外，看起来是不是没什么问题了，那我们来看看表里存储的具体数据：

              id: 3459
            name: fasdfasf
       parent_id: 0
          status: 0
      createtime: 2016-10-20 17:11:12
      modifytime: 2016-10-20 17:11:13
category_id_path: 1,345,2359
1 row in set (0.01 sec)

发现什么问题了么。。

对，category_id_path这个字段，设计者很聪明地存储了一个分类从根到该category的完整路径，但是却犯了个错误。先卖个关子，在这样的表里，如果你要查询345结点(含)的所有子结点要怎么做呢？

select * from category where category_id_path like '1,345%'

这样显然是有问题的。如果一个结点的path是1,3456，那么不幸的，你的查询也会把这个结点选出来。

那么我是不是可以：

select * from category where category_id_path like '1,345,'

看起来似乎是没问题了，那么上面要求的1,345这个结点怎么办呢。。只能

select * from category where category_id_path like '1,345,' or category_id_path = '1,345'

了。倒是问题不大，不过为什么不在path的两边再加一个逗号呢？好处留给读者去思考吧。

12、if/else嵌套层次

从例子开始吧：

func createUser(user UserInfo) {
    if user.Age <= 0 {
        inValid = true;
    } else {
        if user.Name == "" {
            inValid = true
        } else {
            if user.History.Length == 0 {
            } else {
                //500行
                //500行
                //500行
                //500行
                //500行
                //500行
                //500行
            }
       }
   }
   return inValid;
}

把逻辑正常的业务流程放在一个巨大的else里，可能是很多人的爱好。这种情况下如果你在前面的if else有十个，那你可能翻到后面连else里面是什么东西都看不到了(超出了ide的80字的红线)。

改起来很简单：

func createUser(user UserInfo) {
  if user.Age <= 0 {
    inValid = true;
    return inValid
  }
  if user.Name == "" {
    inValid = true
    return inValid
  }

  if user.History.Length == 0 {
    inValid = true;
    return inValid;
  }

  //500行
  return inValid;
}

这样可以让你的代码神清气爽。

13、同一张数据库表的查询，每换一种查询方式就写一个函数

还是我们的线上系统，dao层有这么个mapper：

public interface CategoryMapper xxxx {
    @Select("select * from category where name = #{name}")
    public List<Category> findByName(@Param("name") String name);
    
    @Select("select * from category where id = #{id}")
    public List<Category> findById(@Param("id") Long id);

    @Select("select * from category where parentId = #{parentId}")
    public List<Category> findByParentId(@Param("parentId") Long parentId);

    @Select("select * from category where status = #{status}")
    public List<Category> findByStatus(@Param("status") Integer status);

    //以下略
}

在php的系统里也有类似的东西，model层一张表写了几十个函数，因为我们的表经常会有几十个字段嘛。

写出了这些代码的员工都是我们部门的优秀员工哦~

现在做开发应该尽量避免这种重复劳动。现在不管哪种语言，一般都会有开源的sql builder工具可以直接拿来用。这些sql builder工具一般也会有统一的查询接口，支持传入table name/order by/limit/where的map(java)/array(php)。

这种东西即使你不用开源的，自己开发其实也花不了太久。我们前一家公司的工具就是我们的同事自己完成的，也只有两百行左右。

另外还可以考虑一些代码生成器，dao层如果不是做事务、表关联查询的话，那很多时候几乎不用自己去写这方面的代码了。

如果你不这样做~那么在java里会变成你的dao层噩梦。无数的重复工作的工作量啊。。不过或许老板喜欢能狂怼代码的员工呢~

呵呵。

14、工作流系统update不判断修改前的状态

工作流/订单状态流之类的系统都会有状态流转，其实和编译原理里的状态机意思差不多。不同的状态之间跳转应该有一个基本的先置条件，从某一个固定的状态，满足某一些条件，然后跳转到下一个状态中。但实际的业务系统，却让人发现很多系统在做状态变更的时候，根本不考虑前置状态。比如下面的代码：

update xxx set status = yyy where id = zzz;

像这样，根本不判断前置状态的sql实在是太多了。那么实际运转起来的系统一定不会是你想象的状态流，出现了莫名其妙的跳转你会欲哭无泪。

为了解决这种问题很多系统把校验扔到前端去完成，风险很大。也就是因为这里做的都是内部系统，没有人搞破坏，所以程序员才不重视这些问题。如果你的订单/工单状态流和你公司的奖励、损失密切相关，那我觉得你不会觉得这些不是问题。

解决起来很简单，和经常被提到的高并发时的乐观锁概念差不多，留给读者去思考吧。

15、非单线程系统不考虑线程安全问题

这个问题其实说起来挺复杂的。。多线程程序里很难查的大多是这种问题，所以现在一般做非性能要求很高的系统都会尽量避免掉多线程并发。或者在该并发的时候再并发执行特定的任务，比如java里用future或者ExecutorService之类的来进行数据库并发查询，以减少串行执行任务的等待时间。

不过现在golang成为了搅局者，并发可能会成为未来的常态，所幸的是，golang提供了race检测工具，可以让你方便的在有race的时候程序主动崩溃(233333)。这样好歹是比C的黑暗时代进步太多了。

但在做开发的时候还是应该有这样的概念，如果有全局的map之类的变量，在访问的时候要考虑加锁。加了锁还要评估性能，性能太差还得考虑是不是要参考一些concurrentMap的实现。点点点。

是个蛮复杂的话题。

16、open的资源不关闭，造成句柄泄露

这个错常由php转其它语言的程序员来犯。我们php程序员open的东西从来不close(误。

如果记性不好的话，像数据库连接池，redis连接池之类的资源都会很快被耗尽。然后你就懂了。。

17、抱怨接口性能是语言问题

之前有这么一个程序员，在做了一个要3s才要返回的接口之后说这是贵司php版本5.3的原因，你要是让我换php 7肯定能快十倍。

然后被秒打脸。就是前面说的没有做好批量查询的问题。

这事情有时候确实是。。态度问题。

大多数情况下语言对具体接口的性能影响不会有那么大，所以在你向别人这么说之前，请先简单用日志来记录你的程序每一步所花费的时间为好。

做一个聪明的程序员~

嗯，其实我也是初级程序员。