程序员最头疼的是啥?Bug。那如何写代码才能降低Bug率呢,前辈们已经总结出了各种方法,比较出名的有:设计模式六大原则、Rob Pike六项原则、Unix哲学17条原则、KISS、DRY、Python之禅、宽进严出等等等等,还有各个公司自己的技术栈和编码规则。俗话说的好,道理懂了这么多,为什么还是会写出Bug?孔子曰:“学而不思则惘,思而不学则殆。”道理虽懂,还需实践。
每项原则具体代表什么例子,请自行搜索。
插播一条,小生觉得,中国人写代码,应该是不会出问题的才对。古人云:“各人自扫门前雪,莫管他人瓦上霜。”每个人,每个模块,每行代码负责好自己的事情,整体是个和谐的,划分清晰明了,不会错乱的系统。呵呵,开个玩笑。
下面直接进入例子。
例子1:(实验楼楼赛第15期第1题)
题意说明:算出http请求body的md5值,加到header里(X-Md5),如果没有body,则不加X-Md5。
题意很简单,就是计算http.body的md5,先看实验楼参考答案:
func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
// do something
if req.Body == nil {
return t.RoundTripper.RoundTrip(req)
}
b, err := ioutil.ReadAll(req.Body)
req.Body.Close()
if err != nil {
return nil, err
}
if len(b) ==0 {
return t.RoundTripper.RoundTrip(req)
}
m := md5.Sum(b)
req.Header.Set("X-Md5", hex.EncodeToString(m[:]))
// 由于 ioutil.ReadAll 方法会读取到 EOF,所以需要重置 Body
req.Body = ioutil.NopCloser(bytes.NewBuffer(b))
return t.RoundTripper.RoundTrip(req)
}
代码最后的结果肯定是对的,但这样写真的好吗?有没有感觉作者写的太着急了?从md5.Sum(b)分割,上面一直在操作req.Body,结果req.Body都还没重置呢,就急不可奈的要求md5了,这其实属于“先求结果,慢慢补锅”类型的代码,不提倡,小生更提倡的是,结果固然重要,但不要着急,先把屁股擦干净了,不要隔着长长的一大段业务逻辑填前面的坑。
接下来我们仔细看,先看3个if判断:
- 第1个(第4行)判断req.Body是空,属正常情况,return;
- 第2个(第10行)判断读取req.Body有没有出错,属异常情况,return;
- 第3个(第14行)判断http.Body是空,属正常情况,return。
这里的“正常情况”是指,一个http请求,没有body是很正常的,比如GET请求。
这3个return处,所属情况是不一样的,第1、3处属同一类情况,都是没有请求体http.body,return是一样的。但第2处属异常情况,再加上最后第4个return,我们看到该函数的所有出口处分别为:正常、异常、正常、正常。是不是感觉很奇怪?我们知道,对于一套逻辑,比较好的一种处理流程应该是:异常、异常、异常……正常。所以,小生改写如下:
func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
if req.Body == nil {
req.Body = http.NoBody
}
buf := &bytes.Buffer{}
hs := md5.New()
n, err := io.Copy(hs, io.TeeReader(req.Body, buf))
req.Body.Close()
if err != nil {
return nil, err
}
req.Body = ioutil.NopCloser(buf)
if n > 0 {
req.Header.Add("X-Md5", hex.EncodeToString(hs.Sum(nil)))
}
return t.RoundTripper.RoundTrip(req)
}
判断条件if数量并没有减少,处理流程也一样,但return只剩两个了,按顺序为:异常、正常。
分析一下,第3行,将req.Body为空的情况和len(req.Body)==0的情况合并了;
第7行,该函数的主要目的是求md5,所以将主要目的md5写在了io.Copy(dst, src)中的dst处,而原body不能丢,便将buf放在src处作为原req.Body的备份;
第12行,按逻辑对称的方式,先把上面req.Body的问题扫尾解决完;
最后留下真正要做的逻辑放在最后4行。
小生的写法不一定是最好,但基本做到了逻辑的通用化处理。须知,if判断越多,人理解起来越麻烦,需要记忆辅助的理解也会越多。通用化处理的好处是:第1处if将http.body为空的情况合并,在读下面代码的时候,不需要记忆目前http.body的状态,可以随时拿来就用;io.Copy读http.body过程出错返回是必不可少了,不必多说;最后只留下真正的逻辑求md5放在最后,符合了从异常到正常流程的处理。
例子2:
共用redis添加前缀,用于区分不同应用。
class RedisProxy:
_cmds = ('get', 'set', 'incr')
def __init__(self, prefix, client):
self._prefix = prefix
self._client = client
def __getattr__(self, cmd):
def inner(*args):
if cmd in self._cmds:
if len(args) > 0:
ls = list(args)
ls[0] = self._prefix + ls[0]
args = tuple(ls)
return self._client.__getattr__(cmd)(*args)
return inner
单例模式
foo = RedisProxy('foo_', RedisClient())
这样写对吗?是对的,调用的时候,如foo.set('key', 'value')
,自动转成set foo_key value
,很方便,很简单,业务逻辑不用分心关注前缀问题。
但这种封装方式是不对的,首先,RedisProxy它的主要作用是什么?只是加个前缀,那它本身承载了多少东西?一个prefix必须知道的,client是必须的吗?cmds是必须的吗?
那小生会修改如下:
_prefix = 'foo_'
def format(key):
return _prefix + key
def trim(key):
return key[len(_prefix):] if key[:len(_prefix)] == _prefix else key
是的,只负责拼接前缀,其余的概不负责,client不维护,哪些命令要加前缀也不管,由业务自行决定。想加的调用format,不想加的不调用。这样做,一是方便测试(呃,貌似也不用测试了),二是不需要知道redis有哪些命令,也不需要知道哪些命令要加前缀,三是不需要维护client状态。所有的所有,由业务自己决定。如果业务方出错了,只影响单一的业务,如果封装的多出错了,那基本上整个应用都会有问题。
现在用起来可能麻烦一点,比如:client.set(format(‘key’), ‘value’)。但一般是可以通过封装一个redis key解析专用模块的,该专用模块用于解析redis命令,找出命令中的key、返回值中的key等信息。
一般应用代码可以分为几类:底层、三方库、业务逻辑、存储逻辑、功能模块(分为专用和通用)等,底层和三方库一般不会动,业务逻辑和存储逻辑一般会解耦,功能模块则处于一个比较尴尬的位置。专用功能模块分为业务专用模块和组件专用模块,业务专用模块和业务挂钩,一般通用性较差,主要适用于当前应用;组件专用模块和专用组件挂钩,对不同组件间不通用,但对不同应用间是通用的。通用功能模块则属于多应用可通用的模块,分为功能通用和工具能用。
将需要码农开发的模块(这里不包括底层和三方库)进行分级,按顺序依次为:1.业务逻辑、2.存储逻辑、3.业务专用、4.组件专用、5.功能通用、6.工具通用。不同模块的组合,生成序号最小者模块,比如:存储逻辑+组件专用->存储逻辑,业务专用+能用功能->业务专用。提高通用性的方法一般是:对接口编程。写代码时分析清自己所在级别,目标级别,一级一级逼近,一般写出来的代码都不会太差。
比如上面例子,如果想做一个通用的RedisProxy,它需要是三个不同功能的合集:redis client管理模块、redis key解析模块、和前缀添加删除模块。
伪代码如下:
class RedisProxy:
def __init__(self, client, cmd_parser, key_formater):
self._client = client
self._cmd_parser = cmd_parser
self._key_formater = key_formater
def _format_key(typ, val):
if typ == REDIS_KEY:
return self._key_formater.format(val)
return val
def _trim_key(typ, val):
if typ == REDIS_KEY:
return self._key_formater.trim(val)
return val
def __getattr__(self, cmd):
def inner(*args):
if self._client.state != CONNECT:
self._client.reconnect()
args = self._cmd_parser.command(cmd, args, self._format_key)
resp = self._client.__getattr__(cmd)(*args)
return self._cmd_parser.response(cmd, resp, self._trim_key)
return inner
这样,在用的时候,可以将proxy.set('key', 'val')
翻译成set key val
,再改为set prefix_key val
到redis执行;将proxy.scan(0)
得到的结果(0,('prefix_key'))
改为(0, (‘key’))。这样进去出来的结果是匹配的,就像数据结构里的栈,pop(push(x)) == x
。再看RedisProxy
,它的组成模块中,client
和cmd_parser
属于组件专用模块,key_formater
属于业务专用模块(因为每个应用对键的格式化是不同的),由三个模块组合成了一个最终的业务专用模块。
例子3:
解析形如"a:123,b:456",得到a=123和b=456。
不要把这个问题想复杂,只是个简单的字符串切割问题,你会怎么切?
s = 'a=123,b=456'
ls = s.split(',b=')
a = int(ls[0][2:])
b = int(ls[1])
print(a, b)
这样切对不对呢?从结果上来说是对的,但程序是“侵权”的,split
的任务是按,
切成n份,下面每份要干什么,怎么干,都不是split
该关心的事。意思是,每行代码,搞清楚它要做的事,不要越权,“莫管他人瓦上霜”。上面代码,split
和下面两行代码之间存在着强耦和,另外一个问题是,它不是按规则切的,切割出来的结果存在着不对称性,以对称为美的社会,这样的代码也是不美观的。修改如下:
s = 'a=123,b=456'
ls = s.split(',')
d = {}
for t in ls:
kv = t.split('=')
d[kv[0]] = kv[1]
a = int(d['a'])
b = int(d['b'])
print(a, b)
这样切,每步操作都是在“各扫门前雪”,不会和上下文产生强耦合,而且操作都是对称的。
例子4:
在Golang中,每个http api返回的数据,在结构上都是大体一样的,所以基本上每个项目的入口处都要封装自己的消息返回格式,比如:
type Response struct {
Code int `json:"code"`
Msg string `json:"msg"`
Data interface{} `json:"data"`
}
type JsonRender struct {
RespWriter http.ResponseWriter
}
func (this *JsonRender) RenderErr(code int) {
json.NewEncoder(this.RespWriter).Encode(&Response{
Code: code,
Msg: errcode.Text(code),
Data: "",
})
}
func (this *JsonRender) RenderOK(data interface{}) {
json.NewEncoder(this.RespWriter).Encode(&Response{
Code: 0,
Msg: "",
Data: data,
})
}
这样封装,固然是达到了目的,但这种封装方式对吗?
首先,我们要先问这样一个问题?我们封装返回值的目的是什么?很明显,不用外界关心以何种编码方式返回数据。所以第一点,Response
不应该导出,应该改为type response struct{...}
。
第二,我们再看JsonRender
的作用,只是封装了一个http.ResponseWriter
,而且也是导出字段;两个方法很简短,直接往http.ResponseWriter
里面写json数据;这里不禁要问了:这个封装有什么存在的意义?是方便使用了,还是屏蔽细节了?好像两者都没有。所以这个封装毫无意义可言。
最基本的修改如下:
type response struct {
Code int `json:"code"`
Msg string `json:"msg"`
Data interface{} `json:"data,omitempty"`
}
func render(w http.ResponseWriter, code int, msg string, data interface{}) {
w.Header().Set("Content-Type", "application/json; charset=utf8")
json.NewEncoder(this.RespWriter).Encode(&Response{
Code: code,
Msg: msg,
Data: data,
})
}
func RenderOK(w http.ResponseWriter, data interface{}) {
render(w, 0, "", data)
}
func RenderErr(w http.ResponseWriter, code int) {
render(w, code, errcode.Text(code), nil)
}
只暴露RenderOK
和RenderErr
,里面具体内容不外泄。
例子5:
变量作用域和通道关闭操作
func handleConn(conn net.Conn) {
const QueueSize = 10
mq := make(chan string, QueueSize)
defer close(mq)
go func() {
for {
select {
case msg, ok := <-mq:
if !ok {
log.Printf("debug: msg queue closed")
return
}
_ = msg
// some logic to process msg
}
}
}()
var pkt []byte
var err error
for {
pkt, err = read(conn)
if err != nil {
return
}
go func() {
msg := parsePacket(pkt)
mq <- msg
}()
}
}
上面代码有两个BUG,变量作用域这个BUG比较容易发现,pkt
和err
定义在了for
循环外面,那么就要求,for
循环里面不能有goroutine闭包,其中err
没出现在闭包里,但pkt
出现在了闭包中,并且是go func
中,那这里肯定是个BUG了。
close(chan)
这个BUG好多人觉得不容易发现,但其实这个BUG应该是一眼就看出来的,为什么?因为mq
的定义和defer close(mq)
离得非常近,看到close
操作就该找是谁在写mq
。正常逻辑是:哪个协程在写chan
,就该由哪个协程close(chan)
。由此,我们看到close
,马上找谁在写,很明显在函数最后面的go func
中,这就扎眼了,写chan
的和close(chan)
没在同一协程里,这肯定是要出BUG的。
修正这两个BUG也非常容易:
func handleConn(conn net.Conn) {
const QueueSize = 10
mq := make(chan string, QueueSize)
wg := &sync.WaitGroup{}
defer func() {
wg.Wait()
close(mq)
}()
go func() {
for {
select {
case msg, ok := <-mq:
if !ok {
log.Printf("debug: msg queue closed")
return
}
_ = msg
// some logic to process msg
}
}
}()
for {
pkt, err := read(conn)
if err != nil {
return
}
wg.Add(1)
go func() {
defer wg.Done()
msg := parsePacket(pkt)
mq <- msg
}()
}
}
将pkt
和err
放到for
循环里面去定义;写mq
的有多个协程,那就等所有写协程都退出再close(chan)
。
那有人可能会问,即使加了sync.WaitGroup
,写chan
协程和关闭chan
协程仍然不是同一个啊,那是不是还有BUG?不会,用sync.WaitGroup
后,可以理解为将所有的写操作都转移到了当前协程中,这样就相当于写和关闭在同一协程中了。
例子6:
关闭http.Response.Body
resp, err := http.Get("http://example.com")
if err != nil {
// handle err
}
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
// handle err
}
// use body
_ = body
对于经常写go程序的人来说,上面这段代码BUG非常好找,少了一句defer resp.Body.Close()
,但在这里,我不是要说这个BUG,而是面对这段有问题的代码时,该怎么尽快找到BUG并解决它。
看看以下三篇文章是怎么发现并解决这个问题的吧:
看看这篇拯救发际线的干货吧–警惕 Go 编程陷阱
Go http request 引起的 goroutine 泄漏
Go Http包解析:为什么需要response.Body.Close()
这种分析问题的方式,对吗?完全不对!写代码前,一定要看清楚http.Response.Body
的类型,net/http/response.go
里面写着:
type Response struct {
// ...
Body io.ReadCloser
// ...
}
Body
的类型是io.ReadCloser
,ReadCloser
的用法很简单:Read
完Close
,这么简单的规则,根本范不着读源代码。即便说看到类型,不确定这样用对不对,再看注释也就明白了。注释里写道:It is the caller's responsibility to close Body.
这样就很明显了,读完关闭。
所以,写代码一定要遵循规则,出了问题也不一定非要分析源代码才能解决。
囿于例子不太好找有代表性的,故而以后有发现随时补充。