Golang 中的良好代码与糟糕代码

最近，有人要求我详细解释在 Golang 中什么是好的代码和坏的代码。我觉得这个练习非常有趣。实际上，足够有趣以至于我写了一篇关于这个话题的文章。为了说明我的回答，我选择了我在空中交通管理（ATM）领域遇到的一个具体用例。

背景

首先，简要解释一下实现的背景。

欧洲航空管制组织（Eurocontrol）是管理欧洲各国航空交通的组织。Eurocontrol 与航空导航服务提供商（ANSP）之间交换数据的通用网络称为 AFTN。这个网络主要用于交换两种不同类型的消息：ADEXP 和 ICAO 消息。每种消息类型都有自己的语法，但在语义上，这两种类型是等价的（或多或少）。在这个上下文中，性能必须是实现的关键要素。

该项目需要提供两种基于 Go 解析 ADEXP 消息的实现（ICAO 没有在这个练习中处理）：

一个糟糕的实现（包名：bad）
一个重构后的实现（包名：good）

可以在这里找到 ADEXP 消息的示例。

在这个练习中，解析器只处理了 ADEXP 消息中的一部分字段。但这仍然是相关的，因为它可以说明常见的 Golang 错误。

解析

简而言之，ADEXP 消息是一组令牌。令牌类型可以是：一组令牌的重复列表。每行包含一组令牌子列表（在本示例中为 GEOID、LATTD、LONGTD）。

考虑到这个背景，重要的是要实现一个可以利用并行性的版本。所以算法如下：

预处理步骤来清理和重新排列输入消息（我们必须清除潜在的空格，重新排列多行的令牌，如 COMMENT 等）。
然后在一个给定的 goroutine 中拆分每一行。每个 goroutine 将负责处理一行并返回结果。
最后，收集结果并返回一个 Message 结构。这个结构是一个通用的结构，无论消息类型是 ADEXP 还是 ICAO。

每个包都包含一个 adexp.go 文件，暴露了主要的函数 ParseAdexpMessage()。

逐步比较

现在，让我们逐步看看我认为是糟糕代码的部分，以及我是如何重构它的。

字符串 vs []byte

糟糕的实现仅处理字符串输入。由于 Go 提供了对字节操作的强大支持（基本操作如修剪、正则表达式等），并且考虑到输入很可能是 []byte（考虑到 AFTN 消息是通过 TCP 接收的），实际上没有理由强制使用字符串输入。

错误处理

糟糕的实现中的错误处理有些糟糕。
我们可以找到一些潜在错误返回的情况，而第二个参数中的错误甚至没有被处理：

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

考虑到这个项目的背景（性能很重要），并考虑到消息可能会相当庞大，更好的选择是传递对容器结构的指针。否则，在先前的示例中，每次调用都会复制容器值。

优秀的实现并不面临这个问题，因为它处理切片（无论底层数据如何，都是一个简单的 24 字节结构）。

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

糟糕的实现基于一个很好的初始想法：利用 goroutine 并行处理数据（每行一个 goroutine）。

这是通过在循环遍历行数的过程中，为每一行启动一个 mapLine() 调用的 goroutine 完成的。

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

因为结构中包含一些切片，这些切片可能会被并发地修改（由两个或更多的 goroutine 同时修改），在糟糕的实现中，我们不得不处理互斥锁。

例如，Message 结构包含一个 Estdata []estdata。
通过添加另一个 estdata 来修改切片必须这样做：

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

现实情况是，除非是非常特殊的用例，必须在 goroutine 中使用互斥锁可能是代码存在问题的迹象。

缺点 #2：伪共享

跨线程/协程共享内存并不是一个好主意，因为可能存在伪共享（一个 CPU 核心缓存中的缓存行可能会被另一个 CPU 核心缓存无效）。这意味着，如果线程/协程意图对其进行更改，我们应该尽量避免在线程/协程之间共享相同的变量。

在这个例子中，我认为伪共享影响不大，因为输入文件相当轻量级（在 Message 结构中添加填充字段并进行性能测试得到的结果大致相同）。然而，在我看来，这始终是一件需要牢记的重要事情。

现在让我们看一下好的实现是如何处理并行处理的：

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

现在，mapLine() 只接收两个输入：

当前行
一个通道。这次，这个通道不仅用于在行处理完成时发送通知，还用于发送实际结果。这意味着不应该由 goroutine 来修改最终的 Message 结构。

父 goroutine（生成单独的 goroutine 中的 mapLine() 调用的那个）通过以下方式收集结果：

msg := Message{}

for range in {
    data := <-ch

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

这个实现更符合 Go 的原则，只通过通信来共享内存。Message 变量由单个 Goroutine 修改，以防止潜在的并发切片修改和错误共享。

即使是好的代码也可能面临一个潜在的批评，就是为每一行代码都创建一个 Goroutine。这样的实现可以工作，因为 ADEXP 消息不会包含成千上万行的内容。然而，在非常高的吞吐量下，简单的实现每个请求触发一个 Goroutine 的方式并不具有很强的可扩展性。更好的选择可能是创建一个可重用 Goroutine 池。

编辑： 假设（一行代码 = 一个 Goroutine）绝对不是一个好主意，因为它会导致过多的上下文切换。要获取更多信息，请查看 further reading 章节末尾的链接。

处理行的通知

在不好的实现中，如上所述，一旦通过 mapLine() 完成行处理，我们应该通知父 Goroutine。这是通过使用 chan string 通道和调用来实现的：

ch <- "ok"

对于父 Goroutine 实际上并不检查通道发送的值，更好的选择是使用 chan struct{}，使用 ch <- struct{}{}，甚至更好（对 GC 更友好）的选择是使用 chan interface{}，使用 ch <- nil。

另一种方法（在我看来更清晰的方法）是使用 sync.WaitGroup，因为父 Goroutine 只需在每个 mapLine() 完成后继续执行。

If

Go 语言的 if 语句允许在条件之前传递一个语句。

对于这段代码的改进版本：

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

以下实现可以是这样的：

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

它稍微提高了代码的可读性。

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() 是一个解析复杂标记的函数。糟糕代码中的算法是使用递归完成的：

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 不支持尾递归消除以优化子函数调用。良好的代码产生完全相同的结果，但使用迭代算法：

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（优雅的）iota 的更优雅的解决方案：

const (
    AdexpType = iota
    IcaoType 
)

它产生完全相同的结果，但减少了潜在的开发人员错误。

接收器函数

每个解析器提供一个函数来确定消息是否涉及更高级别（至少有一个路由点在 350 级以上）。

糟糕的代码是这样实现的：

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

    return false
}

意味着我们必须将消息作为函数的输入参数传递。
而良好的代码只是一个带有消息接收器的函数：

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

    return false
}

第二种方法更可取。我们只需指示消息结构实现了特定的行为。

这也可能是使用 Go 接口的第一步。例如，如果将来我们需要创建另一个具有相同行为（IsUpperLevel()）的结构体，初始代码甚至不需要重构（因为消息已经实现了这个行为）。

注释

这是相当明显的，但糟糕的注释写得很糟糕。

另一方面，我尝试像在实际项目中那样注释良好的代码。尽管我不是喜欢每一行都注释的开发者，但我仍然认为至少对每个函数和复杂函数中的主要步骤进行注释是重要的。

举个例子：

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

msg := Message{}

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

除了函数注释之外，一个具体的例子也可能非常有用：

// Parse a line by returning the header (token name) and the value. 
// Example: -COMMENT TEST must returns COMMENT and TEST (in byte slices)
func parseLine(in []byte) ([]byte, []byte) {
    // ...
}

这样具体的例子可以帮助其他开发人员更好地理解现有项目。

最后但同样重要的是，根据 Go 的最佳实践，包本身也应进行注释。

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

package good

日志记录

另一个显而易见的例子是糟糕代码中缺乏生成的日志。因为我不是标准日志包的粉丝，所以在这个项目中我使用了一个名为 logrus 的外部库。

go fmt

Go 提供了一套强大的工具，比如 go fmt。不幸的是，我们忘记在糟糕的代码上应用它，而在良好的代码上已经做了。

DDD

领域驱动设计（DDD）引入了普遍语言的概念，强调了在整个项目参与者（业务专家、开发人员、测试人员等）之间使用共享语言的重要性。在这个例子中无法真正衡量这一点，但保持像 Message 这样的简单结构符合领域边界内部使用的语言也是提高整体项目可维护性的一个好方法。

性能结果

在 i7–7700 4x 3.60Ghz 上，我进行了基准测试来比较两个解析器：

糟糕的实现：60430 纳秒/操作
良好的实现：45996 纳秒/操作

糟糕的代码比良好的代码慢了超过30%。

结论

在我看来，很难给出糟糕代码和良好代码的一般定义。在一个上下文中的代码可能被认为是好的，而在另一个上下文中可能被认为是糟糕的。

良好代码的第一个明显特征是根据给定的功能需求提供正确的解决方案。如果代码不符合需求，即使它很高效，也是相当无用的。

同时，对于开发人员来说，关心简单、易维护和高效的代码也很重要。

性能改进并非凭空而来，它伴随着代码复杂性的增加。

一个优秀的开发人员是能够在特定的上下文中找到这些特性之间的平衡的人。

就像在 DDD 中一样，上下文是关键的 😃