预计阅读时间： 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。
为了提高处理的效率，我们自然希望通过并发的方式来处理这些连续的数据。
所以，基本我们的算法如下：
- 清洗数据（去除空格，去除注释等等）
- 通过
goroutine去分割每一行。每个goroutine处理一行，然后返回结果。 - 最后，把处理后的信息合并起来，生成新的信息。比如
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 = 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()函数是用来解析复杂的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 UL153 VADEM/N0400F320 UN853 PENDU/N0400F330 UN853 IXILU/N0400F340 UN853 DIK/N0400F320 UY37 BATTY
