译者: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 = value
case tokenAdep:
msg.Adep = value
case tokenAltnz:
msg.Alternate = value
// Other cases
}
复制代码
如果开发考虑了所有的情况,default 也是可被选择的。但是,最好捕捉如下的特定情况:
witch 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)
}
复制代码
开发人员处理 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
}
复制代码
评论