近来，经常有人问我在Go 语言的项目里，什么样的代码算好代码，什么样的代码算坏代码。

我发现这个练习很有趣。嗯，这个项目太有趣了，所以我决定写一篇博客和大家分享一下。

为了更好的和大家说明我的观点，我利用一个我工作中碰到的问题为例，给大家讲讲。这个项目是一个空中流量控制系统（ATM）。

代码我放在Github[1]上了。


背景

首先，我简单介绍一下这个项目。

EurocontrolEurocontrolAir Navigation Service Provider (ANSP)AFTNADEXPICAO

其实我也不知道这两类信息有啥区别，不过我们当成两类不同的信息就好了。

每种消息类型有它自己的格式，不过两者传达的信息都是等价的（或多或少）。

我们这个项目最关键的是性能。

ADEXP
  • 一种当然是比较差的实现(包名是bad)
  • 一种是老司机的实现（当然是好的啦，包名自然是good）

我们在这个练习里，只处理其中的一小部分。

我们的主要目的是为了说明问题，不是证明我有多强，对吧？


解析

ADEXP
ARCIDARIDACA878
-EETFIR EHAA 0853
-EETFIR EBBU 0908
FIRFIR
FIREHAA 0853EBBU 0908
GEOGEOIDLATTDLONGTD

为了提高处理的效率，我们自然希望通过并发的方式来处理这些连续的数据。

所以，基本我们的算法如下：
goroutinegoroutineADEXPICAO
adexp.goParseAdexpMessage()


咱们下面一步步来比较一下

让我们现在一步步的来实现这个算法。通过两种方式，我们可以比较一下那种写法更好，那种写法比较差，以及为什么。

String vs []byte

字节BytestrimregexpAFTNTCP

译者注：直接用byte处理会速度快一些

Error 管理

在差的实现的这个包里的代码实现是非常糟糕的。（说的非常好，不然呢）。在第二个参数里返回的一些潜在的错误甚至没有得到处理。

preprocessed, _ := preprocess(string)

好的代码会处理各种潜在的错误：

如下的这种实现也不好，还有错误：

if len(in) == 0 {
  return '', fmt.Errorf('Input is empty')
}
errors.New()

敲黑板啦，所以好的实现应该是这样的:

避免嵌套

译者注：嵌套是初级程序员跨不去的一个坎 ：）

mapLine()
func mapLine(msg *Message, in string, ch chan string) {
    if !startWith(in, stringComment) {
        token, value := parseLine(in)
        if token != '' {
            f, contains := factory[string(token)]
            if !contains {
                ch <- 'ok'
            } else {
                data := f(token, value)
                enrichMessage(msg, data)
                ch <- 'ok'
            }
        } else {
            ch <- 'ok'
            return
        }
    } else {
        ch <- 'ok'
        return
    }
}

相反，好的实现可以尽量地避免嵌套：

用我的观点来看，这样的代码更易读一些。另外，错误处理也最好这样实现。

例如：

a, err := f1()
if err == nil {
    b, err := f2()
    if err == nil {
        return b, nil
    } else {
        return nil, err
    }
} else {
    return nil, err
}

译者注：这可能是烂的不能再烂的代码了

对比一些这个：

再说一遍，这样看起来好多了。

通过传值或引用传递数据

典型的差的实现如下：

func preprocess(in container) (container, error) {
}
container

因为采用了分片（一个简单的忽略了底层数据的24字节的结构）处理，所以好的实现不存在这些问题。

通过传值或传引用的方式传递参数其实是一个和语言无关的事。通过传值这种方式传递参数可以避免一些副作用，比如数据被修改，等等。

译者注：如果大家对这点有点疑惑，需要去做做功课咯。传值一般是复制一份数据，传递到另外的函数去处理。这样的缺点是浪费内存，优点是数据不会修改。传递指针（也就是引用的方式）优点是节约内存，缺点是数据可能被修改。我看很多C++程序员就是故意这样去处理的。。。很费解。有C++程序员站出来解释一下~

传值还有一些优点，比如在单元测试的时候，或者重构并发的代码等等（否则我们需要每次都检查一下数据是否被修改）。

我非常相信具体在选择传值还是引用的问题上，一定要根据项目的背景去权衡。（谁说不是呢，外国人写点东西废话真不少。。。）

并发

goroutinegoroutine
for i := 0; i < len(lines); i++ {
    go mapLine(&msg, lines[i], ch)
}
msg
mapLine()
msgmapLine()msgchannel

给一个共享的Message变量发送指针违反了Go的原则：

不要通过共享内存来通信，通过通信来共享内存。

共享变量有两个缺点：

  • 缺点#1：造成Slice的并发修改

因为可以并发地修改 Slice（两个或两个以上的go routine并发地修改），在差的实现中，我们不得不处理mutex。

例如，Message 包含 Estdata [] estdata。通过append 别的estdata修改slice，一定要这样才行：

事实上，除了一些很特殊的场景，使用在go routine 中使用 mutex 可能本身就是臭代码。

  • 缺点#2：假共享

因为潜在的假共享（一个CPU的core cache的cache line对于别的CPU core cache可能是无效的）的问题，所以通过线程或者goroutine 共享内存不是个好主意。也就是说我们要尽可能地避免在不同的线程或goroutine间共享会被修改的变量。

虽然在这里例子中，因为输入文件很小，所以假共享的作用看不出来。但是，以我来看，最好在开发者牢记这点。

让我们看看好的实现怎么处理并行：

for _, line := range in {
    go mapLine(line, ch)
}
mapLine()
goroutine
goroutinegoroutine

我认为这样实现最好，更符合Go语言仅仅通过通信去共享内存的原则。

spawngoroutinegoroutinegoroutine

行处理结束后通知

goroutine
ch <- 'ok'
chan struct{}ch <- struct{}{}chan interface{}ch <- nil

If

Go 允许在判断条件前加其他语句。

如下的代码，

可以简写一下：

if f, contains := factory[sToken]; contains {
    // Do something
}

经过重构，提高了一点可读性，有没有呢？

Switch

差的实现中，switch没有添加处理默认情况。

虽然说对于开发者来说，默认值是可选的。不过假如有默认值，肯定会更好：

switch simpleToken.token {
case tokenTitle:
    msg.Title = value
case tokenAdep:
    msg.Adep = value
case tokenAltnz:
    msg.Alternate = value
// Other cases    
default:
    log.Errorf('unexpected token type %v', simpleToken.token)
    return Message{}, fmt.Errorf('unexpected token type %v', simpleToken.token)
}

添加默认值，有助于尽可能地避免开发者开发过程中产生的错误。

递归

parseComplexLines()

Go不支持tail-call（尾调用，在函数体末尾返回另一个函数的调用）优化子函数调用。好的实现通过遍历算法达到了同样的结果。

func parseComplexToken(token string, value []byte) interface{} {
    if value == nil {
        log.Warnf('Empty value')
        return complexToken{token, nil}
    }

    var v []map[string]string
    currentMap := make(map[string]string)

    matches := regexpSubfield.FindAll(value, -1)

    for _, sub := range matches {
        h, l := parseLine(sub)

        if _, contains := currentMap[string(h)]; contains {
            v = append(v, currentMap)
            currentMap = make(map[string]string)
        }

        currentMap[string(h)] = string(bytes.Trim(l, stringEmpty))
    }
    v = append(v, currentMap)

    return complexToken{token, v}
}

第二种实现比第一种性能更好。

常量管理

我们需要把ADEXP和ICAO消息分开。差的实现如下：

更优雅的Go实现如下：

const (
    AdexpType = iota
    IcaoType 
)

它们的计算结果是一样的，下面的这种写法可以减少潜在的程序员犯的错。

Receiver function

每个解析器提供了函数来判断消息是否是超过了upper level（至少一个点超过了level 350）。

比较差的代码实现如下：

这意味着我们必须把Message传递给函数。然而好的代码使用Message receiver 简化了函数：


func (m *Message) IsUpperLevel() bool {
    for _, r := range m.RoutePoints {
        if r.FlightLevel > upperLevel {
            return true
        }
    }

    return false
}

下面的这些实现更受人喜欢。我们给Message struct 添加了具体的实现。

它可能也是使用Go interface的第一步。假如有一天我们需要创建具有同样行为（IsUpperLevel()）的另一个struct，甚至不需要重构初始化的代码（因为Message已经implement这个实现了）。

注释

还有一个很明显的问题就是差的代码注释很少。

另一方面，我尽量像我在真实项目中一样注释“好的代码”。即使这样，我也不是那种会注释每一行代码的开发者，我仍然认为在一个复杂的函数里，至少每个函数以及主要的步骤都应该注释一下。

例如：

关于注释一个很有说服力的例子如下：


// 解析一行并返回header（token名）和值
// 例如: -COMMENT TEST 应该返回 COMMENT 和 TEST (in byte slices)
func parseLine(in []byte) ([]byte, []byte) {
    // ...
}

这样的代码真的可以帮助到其他程序员更好的理解现存的项目

雖然最後才說，但不表示它最不重要，根据Go 的最佳实践，package最好也需要注释一下。

日志

另一个显而易见的问题是在差的实现里缺少日志。因为我不喜欢Go标准库的log package，所以我使用了Logrus[2].

go fmt

Go 提供了很好的格式化工具，go fmt。很不幸，我们忘记了在差的实现上执行了（译者注：你确定不是故意的吗？），但是我们再好的代码上执行了。（译者注：好意外啊）；

DDD

（略）

性能比较结果

在一台 i7–7700 4x 3.60Ghz机子上，我分别跑了一下两个解析器：

  • 差的实现：60430 ns/op
  • 好的实现：45996 ns/op

差的代码比好的实现慢30%。


结论

对我来说，其实定义好的实现和差的实现是比较难的。离开场景谈应用都是耍流氓。

好的代码的第一个特征是根据需求提供正确的解决方案。一个不满足需求的代码，哪怕性能再好，也没什么卵用。简单、可维护、性能好的代码是值得开发者时刻关注的几个点。

性能不会凭空提升，它和代码的复杂度增加相关。

好的开发者能够根据需求和这些特点中找到平衡。

和DDD一样，context是关键！：）

同时，对于开发者来说

附录：

ADEXP 文件如下:

-TITLE IFPL
-ADEP CYYZ
-ALTNZ EASTERN :CREEK'()+,./
-ADES AFIL
-ARCID ACA878
-ARCTYP A333
-CEQPT SDE3FGHIJ3J5LM1ORVWXY
-EETFIR KZLC 0035
-EETFIR KZDV 0131
-EETFIR KZMP 0200
-EETFIR CZWG 0247
-EETFIR CZUL 0349
-EETFIR CZQX 0459
-EETFIR EGGX 0655
-EETFIR EGPX 0800
-EETFIR EGTT 0831
-EETFIR EHAA 0853
-EETFIR EBBU 0908
-EETFIR EDGG 0921
-EETFIR EDUU 0921
-ESTDATA -PTID XETBO -ETO 170302032300 -FL F390
-ESTDATA -PTID ARKIL -ETO 170302032300 -FL F390
-GEO -GEOID GEO01 -LATTD 490000N -LONGTD 0500000W
-GEO -GEOID GEO02 -LATTD 500000N -LONGTD 0400000W
-GEO -GEOID GEO04 -LATTD 520000N -LONGTD 0200000W
-BEGIN RTEPTS
       -PT -PTID CYYZ -FL F000 -ETO 170301220429
       -PT -PTID JOOPY -FL F390 -ETO 170302002327
       -PT -PTID GEO01 -FL F390 -ETO 170302003347
       -PT -PTID BLM -FL F171 -ETO 170302051642
       -PT -PTID LSZH -FL F014 -ETO 170302052710
-END RTEPTS
-SPEED N0456 ARKIL
-SPEED N0457 LIZAD
-MSGTXT (ACH-BEL20B-LIML1050-EBBR-DOF/150521-14/HOC/1120F320 -18/PBN/B1 DOF/150521 REG/OODWK RVR/150 OPR/BEL ORGN/LSAZZQZG SRC/AFP RMK/AGCS EQUIPPED)
-COMMENT ???FPD.F15: N0410F300 ARLES UL153 PUNSA/N0410F300 UL153
VADEM/N0400F320 UN853 PENDU/N0400F330 UN853 IXILU/N0400F340 UN853
DIK/N0400F320 UY37 BATTY

原文链接：/good-code-vs-bad-code-in-golang-84cb3c5da49d