真实案例（万字长文）：Bad Code vs Good Code in Golang

预计阅读时间： 23分钟 建议收藏后阅读

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

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

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

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

背景

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

Eurocontrol 是欧洲国家管理空中飞机流量的一个机构。通常在Eurocontrol 和 Air Navigation Service Provider (ANSP)直接交换数据的网络是AFTN。这个网络主要用来交换两种不同的消息类型：ADEXP 和 ICAO。

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

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

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

这个项目会提供两种解析ADEXP的Go 语言实现：

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

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

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

解析

简单来说，ADEXP 消息里包含了一些指令，或者说关键字。比如：

-ARCID ACA878


ARCID的意思是飞行器(AiRcraft IDentifier)的识别码是ACA878。

-EETFIR EHAA 0853-EETFIR EBBU 0908

这行的意思是FIR（Flight Information Region）。

第一行FIR是EHAA 0853, 第二行的是EBBU 0908.

-GEO -GEOID GEO01 -LATTD 490000N -LONGTD 0500000W
-GEO -GEOID GEO02 -LATTD 500000N -LONGTD 0400000W


这里面的指令代码是GEO,GEOID, LATTD, LONGTD。

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

所以，基本我们的算法如下：

---

1. 清洗数据（去除空格，去除注释等等）
2. 通过goroutine去分割每一行。每个goroutine处理一行，然后返回结果。
3. 最后，把处理后的信息合并起来，生成新的信息。比如ADEXP或ICAO.

---

每个包包含一个名为adexp.go的文件。这个文件会把主要的函数ParseAdexpMessage()暴露出来。

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

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

String vs []byte

比较差的写法是通过字符串输入来处理。因为Go提供了强大的字节Bytes操作（如trim、regexp等），我们可以把AFTN消息当成通过TCP获取到的消息,所以我们没有理由把它转换成字符串输入。

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

Error 管理

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

preprocessed, _ := preprocess(string)

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

preprocessed, err := preprocess(bytes)
if err != nil {
  return Message{}, err
}


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

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

第一个错误是语法错误。根据Go语言规范，错误信息首字母不应该大写，结尾也不需要标点符号。因为这个错误信息只是简单的字符串，不需要格式化，所以用errors.New()性能更好。

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

if len(in) == 0 {
 return nil, errors.New('input is empty')
}


避免嵌套

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

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    }}

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

func mapLine(in []byte, ch chan interface{}) {
    // Filter empty lines and comment lines
    if len(in) == 0 || startWith(in, bytesComment) {
        ch <- nil
        return
    }

token, value := parseLine(in)
    if token == nil {
        ch <- nil
        log.Warnf('Token name is empty on line %v', string(in))
        return
    }

sToken := string(token)
    if f, contains := factory[sToken]; contains {
        ch <- f(sToken, value)
        return
    }

log.Warnf('Token %v is not managed by the parser', string(in))
    ch <- nil
}


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

例如：

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

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

对比一些这个：

a, err := f1()
if err != nil {
    return nil, err
}

b, err := f2()
if err != nil {
    return nil, err
}

return b, nil


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

通过传值或引用传递数据

典型的差的实现如下：

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

我们这个项目的背景是性能很重要。另外考虑到处理的数据可能很大，更好的选择是传递container 结构的指针。然而，在上面的例子中，是直接传递数据的。

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

func preprocess(in []byte) ([][]byte, error) {
}


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

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

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

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

并发

这个差的实现其实是基于一个好的想法：通过goroutine去并行化处理数据(每个goroutine处理一行）。

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

这里采用了一个msg 这个共享内存变量。

mapLine() 函数有三个参数：

• 最终返回的消息体msg的指针。mapLine()处理后的数据合并到msg变量。
• 当前处理的行
• 用来发送处理结束通知的channel

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

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

共享变量有两个缺点：

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

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

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

mutexEstdata.Lock()
for _, v := range value {
    fl := extractFlightLevel(v[subtokenFl])
    msg.Estdata = append(msg.Estdata, estdata{v[subtokenPtid], v[subtokenEto], fl})
}
mutexEstdata.Unlock()


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

• 缺点#2：假共享

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

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

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

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

现在mapLine() 有两个输入：

• 当前行
• channel。这次channel不仅会发送一下执行的结果，而且会返回执行的结果。这也意味着goroutine不会修改Message的结构。

处理结果的goroutine的是父goroutine。

msg := Message{}

for range in {
    data := <-ch

switch data.(type) {
        // Modify msg variable
    }
}


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

一个可能被吐槽的点是每行都spawn一个goroutine。之所以这样实现是因为ADEXP 消息只包含几千行，所以也不至于导致goroutine爆发。更好的选择是创建一个可复用的goroutine池。

行处理结束后通知

在差的实现中，就像上面描述的一样，一旦mapLine()处理结束后，我们需要通知父goroutine。这是通过chan string实现的：

ch <- 'ok'

因为父routine没有去检查channel返回的结果，更好的选择可能是使用chan struct{}, ch <- struct{}{}或者其他更好的选择（GC wise），比如chan interface{}，返回ch <- nil。

If

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

如下的代码，

f, contains := factory[string(token)]
if contains {
    // Do something
}


可以简写一下：

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
}


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

switch simpleToken.token {case tokenTitle:    msg.Title = valuecase tokenAdep:    msg.Adep = valuecase 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()函数是用来解析复杂的token。差的实现中使用了递归。

func parseComplexLines(in string, currentMap map[string]string, 
 out []map[string]string) []map[string]string {

match := regexpSubfield.Find([]byte(in))

if match == nil {
        out = append(out, currentMap)
        return out
    }

sub := string(match)

h, l := parseLine(sub)

_, contains := currentMap[string(h)]

if contains {
        out = append(out, currentMap)
        currentMap = make(map[string]string)
    }

currentMap[string(h)] = string(strings.Trim(l, stringEmpty))

return parseComplexLines(in[len(sub):], currentMap, out)
}


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消息分开。差的实现如下：

const (
    AdexpType = 0 // TODO constant
    IcaoType  = 1
)


更优雅的Go实现如下：

const (    AdexpType = iota    IcaoType )

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

Receiver function

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

比较差的代码实现如下：

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

return false
}


这意味着我们必须把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这个实现了）。

注释

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

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

例如：

// Split each line in a goroutine
for _, line := range in {
    go mapLine(line, ch)
}

msg := Message{}

// Gather the goroutine results
for range in {
    // ...
}


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

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

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

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

/*
Package good is a library for parsing the ADEXP messages.
An intermediate format Message is built by the parser.
*/

package good


日志

另一个显而易见的问题是在差的实现里缺少日志。因为我不喜欢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 UL153VADEM/N0400F320 UN853 PENDU/N0400F330 UN853 IXILU/N0400F340 UN853DIK/N0400F320 UY37 BATTY

原文链接：https://teivah.medium.com/good-code-vs-bad-code-in-golang-84cb3c5da49d

作者：Teiva Harsanyi