译者:baiyutang
原文:https://teivah.medium.com/good-code-vs-bad-code-in-golang-84cb3c5da49d
最近,我被问到在 Golang 中,什么是好代码和烂代码。我发现这个训练十分有趣。实际上,很想写一篇博文关于这个话题。先列举这几个要点。
String vs []byte
不好的实现处理是只用 string 入参,我们知道 Golang 提供了一种强大的 bytes 操作的支持(基本操作像 trim、正则表达式等),入参会更多时候是 []byte (考虑到 AFTN 消息是通过 TCP 接收的),实际上并有好的原因强制使用 string 入参。
Error 管理
error 处理是不好的实现很可怕的一种。我们能够发现很多在可能错误返回的地方从没有被处理过。
preprocessed, _ := preprocess(string)
复制代码
好的实现是处理每个可能的错误:
preprocessed, err := preprocess(bytes)if err != nil { return Message{}, err}
复制代码
我们能够发现在一些不好的实现中的错误,像下面的代码:
if len(in) == 0 { return "", fmt.Errorf("Input is empty")}
复制代码
好的实现如下:
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}
复制代码
我觉得这让代码更易于阅读,而且,直接的表达肯定可用在 error 管理上,例如:
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 结构体的指针。否则,在上面的例子中,container 值可能会被复制在每次调用时。
一个好的实现不会遇到这样的难题,因为使用了切片(一个简单的 24 字节结构,而不考虑底层数据 ):
func preprocess(in []byte) ([][]byte, error) {}
复制代码
IF
Go 的 if 语句允许在条件前传递一个语句。
f, contains := factory[string(token)]if contains { // Do something}
复制代码
改良:
if f, contains := factory[sToken]; contains { // Do something}
复制代码
这稍微可以提高代码的可读性。
Switch
另外一个不好的实现错误是忘记 default case,在下面的 switch:
switch simpleToken.token {case tokenTitle: msg.Title = valuecase tokenAdep: msg.Adep = valuecase tokenAltnz: msg.Alternate = value // Other cases}
复制代码
如果开发考虑了所有的情况,default 也是可被选择的。但是,最好捕捉如下的特定情况:
witch 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 case 在尽快捕捉开发期间的潜在 bug 会很有用。
递归
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 不支持尾部调用消除法来优化子函数调用。好的代码可以使用迭代算法来产出准确一致的结果:
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 itota 更优雅的解决方案:
const ( AdexpType = iota IcaoType )
复制代码
这可以产生一样的结果,但可以减少潜在的开发错误。
函数接受者
不好的代码实现如下:
func IsUpperLevel(m Message) bool { for _, r := range m.RoutePoints { if r.FlightLevel > upperLevel { return true } }
return false}
复制代码
意味着我们不得不传递 Message 作为函数的一个入参。
而好的代码是带有一个接受者的简单函数。
func (m *Message) IsUpperLevel() bool { for _, r := range m.RoutePoints { if r.FlightLevel > upperLevel { return true } }
return false}
复制代码
评论