代码重构实战（一）课表查询

2021-11-03 约 5918 字预计阅读 12 分钟

你永远想象不到代码能被重构成什么样子，重构之路充满了惊喜。——我自己

如果不信，请直接跳转到文末。

这本是我交给社团新人的一个重构任务，但后面自己手痒，在他的基础上，又重构了几版。个人自大一波，认为从本次重构中领悟了很多东西，希望也能给读者带来些帮助。本文是基于 Go 语言的，但是在解释思想时，绝大部分内容不涉及特定语法，适合所有开发者阅读。在此感谢 Atom 、Eson 与我进行的讨论、提供的建议，给我带来了非常大的帮助与启发（尤其是代码直观性方面的保持，不要为了重构而重构）。也感谢 Dolt 的初次重构。

一、项目背景介绍

本项目是社团微信公众号的后台代码，本文只关注 schedule 模块：根据微信传来的学号，以及触发词，返回「今日课表」或「明日课表」。

微信端主要是调用了社团上游的课表查询接口，处理初始课表信息，以及封装至微信公众号内。

本模块原始代码因为大佬写得比较赶，所以没有注意封装，大家勿喷。项目其他地方，例如模块可插拔，设计得非常非常优雅，实在佩服。

更确切地说，本次代码重构只针对模块的消息路由注册部分。

二、原始代码

Hexo 貌似没有原生代码折叠语法支持，就先全部展开吧。：）还是 Github 折叠香

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	s.MessageHandler.SetMsgHandlerRouter(string(message.MsgTypeText), "今日课表", server.KeywordTypeEqual, 1, func(msg *server.Message) *message.Reply {
		if msg.GetUserID() == "" {
			return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText("小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~")}
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(m.generateScheduleTodayMessage(msg.GetUserID()))}
	})
	s.MessageHandler.SetMsgHandlerRouter(string(message.MsgTypeText), "明日课表", server.KeywordTypeEqual, 1, func(msg *server.Message) *message.Reply {
		if msg.GetUserID() == "" {
			return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText("小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~")}
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(m.generateScheduleTomorrowMessage(msg.GetUserID()))}
	})

	s.MessageHandler.SetMsgHandlerRouter(message.EventClick, "今日课表", server.KeywordTypeEqual, 1, func(msg *server.Message) *message.Reply {
		if msg.GetUserID() == "" {
			return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText("小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~")}
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(m.generateScheduleTodayMessage(msg.GetUserID()))}
	})
	s.MessageHandler.SetMsgHandlerRouter(message.EventClick, "明日课表", server.KeywordTypeEqual, 1, func(msg *server.Message) *message.Reply {
		if msg.GetUserID() == "" {
			return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText("小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~")}
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(m.generateScheduleTomorrowMessage(msg.GetUserID()))}
	})
}

这串代码，主要传入了两个变量

查哪天的课表：目前有两种可能，今日课表、明日课表。以后可能会加。
支持的微信触发类型：MsgTypeText ：发文字消息触发，MsgTypeClick：菜单点击触发

两两组合，一共组合出了四种消息处理路由。

赶 ddl 赶出来的代码，可重构的点太多，大家都应该有自己的想法，就不讲了。

三、新人重构后

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	setMsgHandler(s, m, "今日课表", message.MsgTypeText)
	setMsgHandler(s, m, "明日课表", message.MsgTypeText)
	setMsgHandler(s, m, "今日课表", message.EventClick)
	setMsgHandler(s, m, "明日课表", message.EventClick)
}

func setMsgHandler(s *server.Server, m *module, key string, msgType message.MsgType) {
	s.MessageHandler.SetMsgHandlerRouter(string(msgType), key, server.KeywordTypeEqual, 1, func() server.MessageHandlerFunc {
		method := m.generateScheduleTodayMessage
		if key == "明日课表" {
			method = m.generateScheduleTomorrowMessage
		}
		return func(msg *server.Message) *message.Reply {
			if msg.GetUserID() == "" {
				return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText("小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~")}
			}
			return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(method(msg.GetUserID()))}
		}
	}())
}

先看看 Dolt 的重构逻辑，简单粗暴地把那四种组合，封装成了一个函数，即 setMsgHandler，传参就传在变动的两个内容：「天数」和「消息」类型。

好处在于，未绑定消息提示只有一份了，以后想改话术不用改多次也不用担心话术不一致问题。以及，封装了重复的函数。

3.1 对新人重构的修改一

先看这段

return func(msg *server.Message) *message.Reply {
	if msg.GetUserID() == "" {
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText("小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~")}
    }
	return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(method(msg.GetUserID()))}
}

先来分析下这段代码做了什么。

判断 userID 是否是空

如果是空，那么就返回 message.Reply 结构体
- 消息设置为未绑定提示
非空，返回 message.Reply 结构体
- 消息设置为格式化后的课表字符串

我认为，上述代码段的两个 return，应当合并为一个，理由如下：

从代码阅读者视角说

首先，后面阅读该代码的人，需要先明白这是个 if else 结构。然后，正常人，会非常仔细地对比这两个 return message.Reply 内的结构体元素，是否完全一致，到底是哪个元素发生了变化。光是这两行代码的元素对比，起码就要耗时 10 秒。

既然代码编写者知道这两行函数只有最后发送的消息不同，那么为什么在写代码的时候就体现出来？而不是说，在其他人日后阅读代码的时候，一个一个元素对比。我觉得这非常不优雅。

从代码整洁度视角说

代码发生了大幅度相似、重复，整洁度不言而喻。

以及，未绑定信息提示太长了，已经超出 80 列了，建议定义成一个常量，传入函数调用中。

我是这么改的：

unBind := "小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~"
var messageReply string
if msg.GetUserID() == "" {
	messageReply = unBind
} else {
	messageReply = scheduleString
}
return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(messageReply)}

首先从感官上，这段代码，没有被浏览器渲染成两行，看起来非常舒适。

然后，读代码的时候，不会有任何歧义，也不会有干瞪眼比对元素是否相同的情况。

3.2 对新人重构的修改二

再看看这段

method := m.generateScheduleTodayMessage
if key == "明日课表" {
	method = m.generateScheduleTomorrowMessage
}

相信读者看到这段的感受都是迷惑的，看了半天，才反映到，这是个 if else。如果传 今日课表 ，就调用查询今天课表的函数，如果传 明日课表 ，就调用查询明天课表的函数。

他先定义了一个默认值 今日课表，然后用 if 判断，如果命中了就覆盖。

如果后来的调用者，传了个 后天课表 ，而没有注意到这个函数的内部实现，那么，就崩了啊，bro~ 最后返回的，是 今日课表 。

如果改成这样呢：

if key = "今日课表" {
    method := m.generateScheduleTodayMessage
} else {
	method = m.generateScheduleTomorrowMessage
}

如果后面的调用者，又没看你函数的内部实现，传了个 后天课表，又炸了呀，宝友，这可不兴写！

你返回的，会是，明天的课表。

所以我个人推荐，所有的已知的判断条件，全部手动硬匹，else 只用来处理未知情况。代码如下：

if key == "今日课表" {
    method := m.generateScheduleTodayMessage
} else if key == "明日课表" {
	method = m.generateScheduleTomorrowMessage
} else {
    // 报错或直接返回
}

这样一来，如果有人调用函数，传了个非法值，会直接收到报错。

3.3 对新人重构的修改三

这段涉及到 Go 的语法了，不会 Go 的读者可以跳过。

新人的问题主要在于，写了无必要的函数嵌套。本来只要返回个函数就行，而他写了个匿名函数来返回这个函数。不但增加了无必要的代码层级，还大大降低了可读性。（我太菜了，那天 review 了半天，才注意到后面有个 ()）

怎么改的话，直接贴个示意图吧，应该能看懂。

四、继续重构？

按照之前的修改，改完后是这样的：

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	setMsgHandler(s, m, today, message.MsgTypeText)
	setMsgHandler(s, m, tomorrow, message.MsgTypeText)
	setMsgHandler(s, m, today, message.EventClick)
	setMsgHandler(s, m, tomorrow, message.EventClick)
}

func setMsgHandler(s *server.Server, m *module, key string, msgType message.MsgType) {
    var method func(string)string // 通过学生 id 获取课表字符串的函数签名
    if key == "今日课表" {
        method := m.generateScheduleTodayMessage
    } else if key == "明日课表" {
        method = m.generateScheduleTomorrowMessage
    } else {
    // 报错或直接返回
	}
	msgHandleFunc := func(msg *server.Message) *message.Reply {
		unBind := "小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~"
		var messageReply string
		if msg.GetUserID() == "" {
			messageReply = unBind
		} else {
            messageReply = method(msg.GetUserID())
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(messageReply)}
	}
	s.MessageHandler.SetMsgHandlerRouter(string(msgType), key, server.KeywordTypeEqual, 1, msgHandleFunc)
}

还能再优化吗？

其实在重构这个文件前，我已经重构过本模块的另外一份代码，其中采纳了 Atom 的建议，感谢。

项目中，生成今日课表或明日课表的代码，其实是这样的：

之前对今日课表和明日课表的查询是完全独立的两个函数，在这次重构之前，我重构了课表消息缝合函数（就是前文说的两个获取今日课表获取明日课表函数）。写了个通过日期偏移，获取任何一天课表的函数。dayOffset 是我为了增强可读性而定义的新类型，本质就是 int，没学过 Go 的读者可以这么理解。

当时为了不影响调用方（即今天重构的消息路由注册函数），只改了函数体，函数头保留了。

今天算是看完了整个模块。既然消息路由的传参是日期偏移，同学查课表的参数也是偏移，那么为什么不统一一下？这样的话，不但能删除这两个「生成今日课表」、「生成明日课表」的函数，以后如果要增加支持课表查询的天数，也不需要新增函数。

改完之后是这样的：

func setMsgHandler(s *server.Server, m *module, offset dayOffset, msgType message.MsgType) {
	msgHandleFunc := func(msg *server.Message) *message.Reply {
        if dayOffset == today {
        	key = "今日课表"
    	} else if dayOffset == tomorrow {
        	key == "明日课表"
    	} else {
    		// 报错或直接返回
		}
		unBind := "小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~"
        // 这里以后就不用再写 if 了
		scheduleString := m.generateScheduleMessageByOffset(msg.GetUserID(), offset)
		var messageReply string
		if msg.GetUserID() == "" {
			messageReply = unBind
		} else {
			messageReply = scheduleString
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(messageReply)}
	}
	s.MessageHandler.SetMsgHandlerRouter(string(msgType), key, server.KeywordTypeEqual, 1, msgHandleFunc)
}

五、还能再优化吗

对于大部分人来说，优化到现在就差不多了。

但我们现在再从程序的拓展修改角度，或者有点像，「开闭原则」，评价一下这个程序。

看看上面这段代码，setMsgHandler 内有一个将 dayOffset 的数值转化成中文 今日课表、明日课表 的判断。

如果我们想增加或删减课表查询的天数，比如增加「明日课表」，必须要对 setMsgHandler 进行一个修改。

这又有个同步问题，如果新开发者调用了 setMsgHandler 函数，传入了 后天 的 dayOffset ，却没有修改函数内 dayOffset → 中文触发词（如 后天课表）的转化。

那就，又寄了啊，姐妹！

我们能不能做到，在拓展功能的情况下，不修改 setMsgHandler 函数？

即，后面的调用者，能不能把 setMsgHandler，当成一个类似于 包内函数 的东西，完全不用关心内部实现，调用的时候不做任何修改？并且不会出现任何同步问题？

很简单，维护一个键为 dayOffset，值为 今日课表、明日课表 等的 map 就行。函数体内，只要实现 map[key] 语法就好。

通用实现是用 map，但 Go 有更优雅的方式，用接口。

当前版本完整代码如下：

（上下两部分在两个文件源文件内，为了阅读方便，函数放到了一起）

const (
	today dayOffset = iota
	tomorrow
)

// 这里就是我所谓的 Go 的接口实现
func (offset dayOffset) String() string {
	return map[dayOffset]string{
		today:    "今日课表",
		tomorrow: "明日课表",
	}[offset]
}

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	setMsgHandler(s, m, today, message.MsgTypeText)
	setMsgHandler(s, m, tomorrow, message.MsgTypeText)
	setMsgHandler(s, m, today, message.EventClick)
	setMsgHandler(s, m, tomorrow, message.EventClick)
}

func setMsgHandler(s *server.Server, m *module, offset dayOffset, msgType message.MsgType) {
	msgHandleFunc := func(msg *server.Message) *message.Reply {
		unBind := "小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~"
		scheduleString := m.generateScheduleMessageByOffset(msg.GetUserID(), offset)
		var messageReply string
		if msg.GetUserID() == "" {
			messageReply = unBind
		} else {
			messageReply = scheduleString
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(messageReply)}
	}
    // offset 调用 String 接口，实现类似 map 效果，直接获得 天数偏移对应的  中文
	s.MessageHandler.SetMsgHandlerRouter(string(msgType), offset.String(), server.KeywordTypeEqual, 1, msgHandleFunc)
}

六、什么，还能更优雅？

继续从拓展与修改的角度来看，我们来看看 Server 函数，这个函数是我们之前四个路由注册函数的调用方。

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	setMsgHandler(s, m, today, message.MsgTypeText)
	setMsgHandler(s, m, tomorrow, message.MsgTypeText)
	setMsgHandler(s, m, today, message.EventClick)
	setMsgHandler(s, m, tomorrow, message.EventClick)
}

确实以后不用修改 setMsgHandler 了，但是每次，还需要加两行。比如加个 后天课表功能，我们需要破坏 Serve 函数，加一行日期为后天、消息类型为文本(msgTypeText)的函数调用，再加一行日期为后天，消息类型为点击(msgTypeText)的函数调用。

这么多行长得相似的，重复了！

怎么优化？！

最简单的，数组！

Show your the code.

// 微信课表查询接口支持的天数，可跨天添加
var supportDays = []dayOffset{today, tomorrow}

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	for _, supportDay := range supportDays {
		setMsgHandler(s, m, supportDay, message.MsgTypeText) // 发消息触发
		setMsgHandler(s, m, supportDay, message.EventClick)  // 点击触发
	}

}

以后再要删改支持查询的天数，只要删改常量，改一下 String() 接口，再把需要支持的天数放到 supportDays 切片（数组）里就好。

七、再来点性能优化？

这部分的优化，是 Atom 提出的建议，涉及一点项目细节，但读者应该能理解。

我们关注 setMsgHandler 中的一小段：

scheduleString := m.generateScheduleMessageByOffset(msg.GetUserID(), offset)
var messageReply string
if msg.GetUserID() == "" {
messageReply = unBind
} else {
messageReply = scheduleString
}
return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(messageReply)}

我来解读一下，第一行是调用了获取课表字符串的接口，这个接口是请求上游服务器的，是本模块最耗时的操作。

但是，当 userID 为空时，并不会用到课表信息，白请求了一次网络。所以我们调用一下查询课表和判断 userID 是否为空的顺序。

于是整个代码就成了这样：

func setMsgHandler(s *server.Server, m *module, offset dayOffset, msgType message.MsgType) {
	msgHandleFunc := func(msg *server.Message) *message.Reply {
		var messageReply string
		userID := msg.GetUserID()
		if userID == "" {
			messageReply = "小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~"
		} else {
            // 当 userID 不为空时，才发起网络请求
			messageReply = m.generateScheduleMessageByOffset(userID, offset)
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(messageReply)}
	}
	s.MessageHandler.SetMsgHandlerRouter(string(msgType), offset.String(), server.KeywordTypeEqual, 1, msgHandleFunc)
}

八、再加点细节？

这个时候，Eson 又说，你的 setMsgHandler ，关于 userID 的内容，足足占了四行，还有 if ，能不能优化一下？

？？？

我们先来分析一下，我特地声明了一个 userID 变量，是因为 userID 在「判空」和「课表查询」的时候都用到了。与其调用两次函数，不如调用一次然后保存。

这怎么优化？

Eson 说，或许能利用被调用函数抛出 error？

懂了，瞬间懂了。

光速改了，贴代码。还是贴图吧，Goland 的代码高亮比较阳间。

用 generateScheduleMessageByOffset 处理空 ID 确实是优雅的，这样能保证它再次被别人调用的时候，被调皮小子传了空 ID ，能返回错误。

九、去除同步依赖

现在的代码还是有一点相互依赖存在的，如果后面的维护者没注意到要同时更新 supportDays 数组和 String 函数内部的 map 中的触发词，会导致某天的菜单为空。能通过编译，运行却可能会出问题。

本来打算的一个实现方式是，在运行时检查所有的触发词是否缺失，如果缺了报 Fatal。因为目前对项目不了解，以为只有当新项目跑起来了，旧项目才会终止（有点像CI那味道，错了就不给部署，我的想法）。我的初衷是，把一切需要约定俗成的东西，强制由编译器检查，把错误扼杀在摇篮里。既然原作者希望后来的开发者遵守该规则，甚至是一定要遵守的，那么，为什么不直接用手段强制检查呢？哪天有个开发者没遵守，程序不就直接崩了？

经 Atom 提醒，如果新项目启动不了，旧项目依然会终止，目前社团部署策略不存在我假想的那种情况。

我仍然觉得，我的重构是有意义的。之前的代码，要改一堆函数，你只能靠后面开发者优秀的素质来支持项目的健壮性，哪个人漏改一个地方，项目就崩了。或者说，项目全是隐患。

可读性、易于拓展、健壮性，是我最关注的地方。然后再最大程度兼顾直观性。

十、最后重构后的效果

这次代码的改动比较小，主要把 String 接口换成了 map，其他函数略微改动了一下。

type dayOffset uint8

const (
	// 天数常量偏移和 uint 数字一一对应
	today    dayOffset = 0
	tomorrow dayOffset = 1
)

// 微信课表查询接口支持的天数，可跨天添加, 右侧的中文是微信菜单和文字消息的触发词
var supportDays = map[dayOffset]string{
	today:    "今日课表",
	tomorrow: "明日课表",
}

func (m *module) Serve(s *server.Server) {
	if _, err := m.cron.AddFunc("5 0 * * *", m.updateTime); err != nil {
		panic(err)
	}
	m.cron.Start()

	// 依次取出支持的查询天数和对应的查询触发词
	for supportDay, trigger := range supportDays {
		setMsgHandler(s, m, supportDay, trigger, message.MsgTypeText) // 发消息触发
		setMsgHandler(s, m, supportDay, trigger, message.EventClick)  // 点击触发
	}

}

func setMsgHandler(s *server.Server, m *module, offset dayOffset, trigger string, msgType message.MsgType) {
	msgHandleFunc := func(msg *server.Message) *message.Reply {
		msgReply, err := m.generateScheduleMessageByOffset(msg.GetUserID(), offset)
		if err != nil {
			msgReply = "小助手无法获得您的绑定信息呢，请确认微信绑定情况，如有疑问加入杭电助手答疑群咨询哦~"
		}
		return &message.Reply{MsgType: message.MsgTypeText, MsgData: message.NewText(msgReply)}
	}
	s.MessageHandler.SetMsgHandlerRouter(string(msgType), trigger, server.KeywordTypeEqual, 1, msgHandleFunc)
}

10.1 简单总结一下

之前想要添加新的支持的课程查询天数，要改一堆东西，各种变量和函数，极容易出错。

现在新建个日期偏移常量，和其对应的中文一起加到 map 就能实现支持任意正向天数的课表查询。除非要改内部实现，否则完全不用改动其他地方的代码。

十一、最后再说一句

优雅啊，哥哥！

目录