最近我被問到關於 `Golang` 好代碼和不好的代碼的一些細節問題，我發現這個問題非常有趣，有趣到可以就這個問題寫一篇文章。為了闡明我的觀點，我舉了一些我在 `Air Traffic Management`（`ATM`）航空交通管制領域的代碼例子，這些代碼託管在 [Github](https://github.com/teivah/golang-good-code-bad-code) 上。## 背景首先，簡短地描述一下項目的背景。`Eurocontrol` (歐洲航管組織)是一個管理歐洲各國航空交通的組織。`Eurocontrol` 和 `Air Navigation Service Provider` (`ANSP`) 之間共同的資料交換網路稱為 `AFTN`。這個網路主要用來交換兩種格式的資訊： `ADEXP` 和 `ICAO`(國際民航組織)格式資訊。每一種資訊都有它們自己的文法格式，但在語義上，或多或少它們是相同的。在這個背景下，效能必須是實現的關鍵因素。這個項目需要提供基於 `GO` 語言的用於轉換 `ADEXP` 格式資訊的兩種實現( `ICAO` 格式的轉換並沒有體現在這次對比中)：- 不好的實現方式（ `package` 包名：[bad](https://github.com/teivah/golang-good-code-bad-code/tree/master/bad)） - 重構之後的實現方式（ `package` 包名：[good](https://github.com/teivah/golang-good-code-bad-code/tree/master/good)） - `ADEXP` 格式的資訊例子可以在[這裡](https://raw.githubusercontent.com/teivah/golang-good-code-bad-code/master/resources/tests/adexp.txt)找到。這次對比的結構裡，在 `ADEXP` 資訊中，解析處理部分只是一個子集，至今它仍然跟 `Go` 中常見的一些錯誤有關係。## 解析概括地講，一個 `ADEXP` 資訊是一個 `tokens` 的集合，可以是下面任意一種：- 簡單的> -ARCID ACA878代表 `ARCID` (航班 `ID` ) 是 `ACA878`。- 迴圈的> -EETFIR EHAA 0853 > -EETFIR EBBU 0908這個例子代表的是一個 `FIR` (飛行情報區)列表，第一個航班是 `EHAA 0853`,第二個 `EBBU 0908`。- 複雜的> -GEO -GEOID GEO01 -LATTD 490000N -LONGTD 0500000W > -GEO -GEOID GEO02 -LATTD 500000N -LONGTD 0400000W一個迴圈的 `tokens` 列表，每一行包含一個子 `token` 列表（在這個例子中是 `GEOID`， `LATTD`， `LONGTD`）。結合項目背景，實現一個並存執行的轉化代碼變得很重要。所以有以下演算法：- 實現一個對輸入進行清理和重新排列的預先處理過程（我們需要清除可能存在的空格，重新排列例如 `COMMENT` 等的多行`tokens`）- 然後分割每一行至一個協程中。每一個協程將會負責處理一行 `tokens`，並且返回結果。- 最後一個步驟同樣重要，整合結果並且返回一個 `Message` 訊息)結構體，這是一個公用的結構，不管是 `ADEXP` 還是 `ICAO` 類型的資訊。每一個包包含一個 `adexp.go` 檔案暴露主要的 `ParseAdexpMessage()` 方法。## 逐步對比現在我們逐步來看下我認為的不好的代碼，並且我是如何重構它的。## String類型 vs []byte類型限制輸入類型為字串類型並不好。`Go` 對 `byte` 類型的處理提供了強大的支援（例如基礎的 `trim`, `regexp` 等），並且輸入將會很大程度上類似於 `[]byte` (鑒於 `AFTN` 資訊是通過 `TCP` 協議來接收的)，實際上沒有理由強制要求字串類型的輸入。## 錯誤處理錯誤的處理實現有點糟糕。我們會發現忽視了在第二個參數中返回的一些可能存在的錯誤：```gopreprocessed, _ := preprocess(string)```好的實現方式是捕獲每一個可能的錯誤：```gopreprocessed, err := preprocess(bytes)if err != nil {return Message{}, err}```我們可以在下面這種不好的代碼中也能找到犯的一些錯誤：```goif len(in) == 0 {return "", fmt.Errorf("Input is empty")}```第一個錯誤是文法上的。根據Go的文法，錯誤提示的字串既不是大寫也不是以標點符號結尾。第二個錯誤是如果一個錯誤資訊是一個簡單的字串常量（不需要格式化），使用輕量的 `errors.New()` 效能會更好。好的實現如下：```goif len(in) == 0 {return nil, errors.New("input is empty")}```## 避免嵌套`mapLine()` 函數就是一個很好的例子來說明避免嵌套調用。不好的代碼如下：```gofunc 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}}```相反，好的代碼錶示得很清晰：```gofunc mapLine(in []byte, ch chan interface{}) {// Filter empty lines and comment linesif len(in) == 0 || startWith(in, bytesComment) {ch <- nilreturn}token, value := parseLine(in)if token == nil {ch <- nillog.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}```在我看來，這使得代碼易讀性更強。此外，這種扁平化的處理方式也應該加到錯誤捕獲代碼中，下面的例子：```goa, err := f1()if err == nil {b, err := f2()if err == nil {return b, nil} else {return nil, err} } else {return nil, err}```應該被修改成：```goa, err := f1()if err != nil {return nil, err}b, err := f2()if err != nil {return nil, err}return b, nil```同樣，第二段代碼的可讀性更好。## 傳值採用value還是reference預先處理方法的實現並不好：```gofunc preprocess(in container) (container, error) {}```結合項目的背景來說(考慮效能)，考慮到一個資訊的結構體有可能會比較大，更好的方式是在`container`結構內傳入指標，否則，在上面例子的代碼中`container`的值將會在每一次調用的時候被覆蓋掉。好的實現代碼將不會有這個問題因為它單個的處理方式(一個簡單的24位元組的結構，不管什麼類型資料)。```func preprocess(in []byte) ([][]byte, error) {}```更廣泛地說，無論是根據引用還是數值傳遞參數都不是一個符合語言習慣的用法。通過數值傳遞資料也能協助確定一個方法將不會帶來任何的副作用（就像在函數的輸入中傳遞資料一樣）。這樣做有幾個好處，例如單元測試、在代碼並發上的重構（否則我們需要檢查每個子函數來確定傳遞是否完成）我確信這種寫法需要根據實際項目背景小心地使用。## 並發不好的實現方式源於一個最初的好的想法：利用協程並發處理資料（一個協程處理一行）。這導致了在一個協程裡反覆調用`mapLine()`。```for i := 0; i < len(lines); i++ {go mapLine(&msg, lines[i], ch)}````mapLine()`方法三個參數：- 返回指向最後一個`Message`結構的指標。這意味著每個`mapLine()`將會被同一個變數填充。- 當前行- 一個`channel`通道用於處理行完成時發送訊息為了共用`Message`訊息而去傳遞一個指標違背了Go基本原則：> 不要通過共用記憶體來通訊，而應該通過通訊來共用記憶體傳遞共用的變數有兩個主要的缺點：- 缺點 #1：分割一致的修飾因為結構中包含一些切片可以被同時修改（同時被兩個或者更多的協程）我們得處理互斥的問題。例如，`Message`訊息結構包含一個`Estdata []estdata`，通過加上另一個`estdata`修改這部分必須像下面這樣處理：```gomutexEstdata.Lock()for _, v := range value {fl := extractFlightLevel(v[subtokenFl])msg.Estdata = append(msg.Estdata, estdata{v[subtokenPtid], v[subtokenEto], fl})}mutexEstdata.Unlock()```事實上，排除特定用法，在協程中使用`mutex`(互斥鎖)並不是一個好的選擇。- 缺點 #2：偽共用通過線程或者協程分享記憶體並不是一個好的方式因為可能存在偽共用（一個在`CPU`緩衝中的進程可以被另一個`CPU`緩衝)。這意味著我們需要儘可能地避免通過線程和協程來共用那些需要修改的變數。在這個例子中，雖然，我不認為偽共用在輸入的檔案教少的情況下有一個很大的影響（在`Message`訊息結構體中增加檔案的效能測試結果或多或少是一樣的），但在我看來這也是一個很重要的需要牢記的點。現在讓我們來看下好的並發處理：```gofor _, line := range in {go mapLine(line, ch)}```現在，`mapLine()`只接受兩個參數：- 當前行- `channel`通道，當前行處理完後，這裡的通道不再簡單地用來發生訊息，也用來傳送實際的結果。這意味著不應該使用協程去修改最後的訊息結構。結果在父級的協程中整合。（產生的`mapLine()`方法在各自的協程中被調用）```gomsg := Message{}for range in {data := <-chswitch data.(type) {// Modify msg variable}}```這個代碼更加一致，在我看來，根據`Go`的原則：通過通訊來共用記憶體。通過單一的協程來修改訊息變數防止了可能由於並發導致的修改和偽共用問題。這部分代碼潛在的問題是造成了每一行一個協程，這種實現將能夠運行，因為`ADEXP`資訊行數將不會太大，在這個簡單實現中，一個請求將產生一個協程，在產生能力上將無法考量。一個更好的選擇是建立一個協程池來複用協程。## 行處理通知在上面不好的代碼中，一旦`mapLine()`處理完一行，我們需要在父級的協程中進行標識。這部分將通過使用`chan string`通道和方法的調用：```goch <- "ok"```因為父級協程並不會檢查通道傳過來的結果，較好的處理是通過`chan struct{}`使用`ch <- struct{}{} `，或者更好的選擇(`GC`會更差)是通過`chan interface{}`使用`ch <- nil`處理。另一個類似的方法（在我看來甚至會更簡潔）是使用`sync.WaitGroup`，因為當每一個`mapLine()`執行完了，父級的協程還需繼續運行。## If條件判斷在`Go`的條件判斷語句中，允許在條件前進行賦值。一個改進版的代碼：```gof, contains := factory[string(token)]if contains {// Do something}```如下：```goif f, contains := factory[sToken]; contains {// Do something}```這樣代碼的可讀性更高。## Switch選擇代碼中犯得另一個錯誤是沒有設定`switch`中的`default`選項：```goswitch simpleToken.token {case tokenTitle:msg.Title = valuecase tokenAdep:msg.Adep = valuecase tokenAltnz:msg.Alternate = value // Other cases}```如果開發人員能夠考慮到所有的情況，`switch`的`default`項是可選的，但是像下面這樣捕獲特殊的情況肯定會更好。```goswitch 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)}```處理`default`選項會協助開發人員捕獲開發過程中可能造成的`bugs`。## 遞迴`parseComplexLines()`是一個解析複雜`token`的方法，在不好的代碼中是使用遞迴來處理：```gofunc 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`不支援尾調用最佳化遞迴，好的代碼使用迭代演算法能得到同樣的結果：```gofunc parseComplexToken(token string, value []byte) interface{} {if value == nil {log.Warnf("Empty value")return complexToken{token, nil}}var v []map[string]stringcurrentMap := 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`類型的訊息。不好的寫法如下：```goconst (AdexpType = 0 // TODO constantIcaoType = 1)``` 反之，利用`Go`的`iota`（常量計數器）能寫出更優雅的代碼：```goconst (AdexpType = iotaIcaoType )```輸出的結果是一致的，但是規避了可能存在的錯誤。## 接收方法每個轉換提供一個函數來判斷每個 `Message` 是否涉及到上一個層級（至少在 350 級以上的路線點）不好的代碼如下：```gofunc IsUpperLevel(m Message) bool {for _, r := range m.RoutePoints {if r.FlightLevel > upperLevel {return true}}return false}```意味著我們必須把 `Message` 當函數的參數。反之，好的代碼是提供一個簡單的函數作為 `Message` 的接收者。```gofunc (m *Message) IsUpperLevel() bool {for _, r := range m.RoutePoints {if r.FlightLevel > upperLevel {return true}}return false}```第二種方法是更可取的，我們簡單地讓訊息實現了一個特定的行為。這也是使用 `Go` 實現介面的第一步，如果某天我們需要建立另一個同一行為 (`IsUpperLevel()`) 的結構，這部分代碼不需要被重構（因為 `Message` 已經繼承該行為）。## 注釋注釋這部分的問題很明顯，但是不好的注釋很少被提及。另一方面，在真實項目中我嘗試著去做很好的注釋，儘管我並不是那種每行都會注釋的人，我依然相信至少在每個函數和在一個複雜的函數的核心部分寫上注釋是非常重要的事情。舉個例子：```go// Split each line in a goroutinefor _, line := range in {go mapLine(line, ch)}msg := Message{}// Gather the goroutine resultsfor range in {// ...}```在注釋中提供一個另外的例子也會提供很大的協助：```go// 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`的最佳實務，包本身也需要注釋。```go/*Package good is a library for parsing the ADEXP messages.An intermediate format Message is built by the parser.*/package good```## Tlog另一個很明顯的問題是在缺少Tlog。因為我並不喜歡 `Go` 提供的標準日誌 `package` 包，我在這個項目中使用一個叫 `logrus` 第三方的日誌包。## go 的fmt包`Go` 提供一個強力的工具集 `go fmt`。遺憾的是我們忘記去利用它。## DDD`DDD` 帶來了通用語言的概念，以強調所有項目之間共用語言的重要性( `business experts`, `dev`, `testers` 等)。這點在這個項目上可能無法衡量，但是從整個項目的可維護性上來考慮，保持一個簡單的相容上下文結構的 `Message` 也是重要的一點。## 效能結果在 `i7-7700 4x 3.60Ghz` 的硬體環境下，我通過基準測試來對比兩種代碼：- 好的代碼： 60430 ns/op- 不好的代碼： 45996 ns/op不好的代碼比好的代碼慢了超過30%。## 結論在我看來，要對好的代碼和不好的代碼給出一個普遍的定義非常難。在一個環境中，一段代碼可以被認為是好的代碼，但在另一個環境中可能被認為是不好的代碼。一個很明顯的特徵就是好的代碼在給定的環境中能很好地解決問題，如果代碼高效，但是不滿足需求，那也是徒勞。同時，考慮代碼的簡潔、可維護性、高效對開發人員來說非常重要。> 效能的提升伴隨著代碼複雜性的增加。一個好的開發人員能夠在給定的環境中在上面這些特徵裡找到一個平衡。就像在`DDD`裡,`context`就是解決方案。
via: https://teivah.io/blog/good-code-vs-bad-code-in-golang/
作者：Teivah Harsanyi 譯者：M1seRy 校對：polaris1119
本文由 GCTT 原創編譯，Go語言中文網 榮譽推出
本文由 GCTT 原創翻譯，Go語言中文網 首發。也想加入譯者行列，為開源做一些自己的貢獻嗎？歡迎加入 GCTT！
翻譯工作和譯文發表僅用於學習和交流目的，翻譯工作遵照 CC-BY-NC-SA 協議規定，如果我們的工作有侵犯到您的權益，請及時聯絡我們。
歡迎遵照 CC-BY-NC-SA 協議規定 轉載，敬請在本文中標註並保留原文/譯文連結和作者/譯者等資訊。
文章僅代表作者的知識和看法，如有不同觀點，請樓下排隊吐槽
704 次點擊
