此为摘自 ==淘宝前端团队(FFD)==
编写可读的代码:
对于以代码为生的程序员而已,编写可读的代码,是一件极其重要的事情。从某种角度来说,代码最重要的功能是能够被阅读,其次才是能被正确执行。一段无法正确执行的代码,也许会使项目延误几天,但造成的危害只是暂时和短暂的,而一段缺乏条理,难以阅读的代码,造成的危害却是深远而长久的。这里总结了下,在工作和业余生活中,我对如何编写可读代码这个问题的具体体会。
- 变量命名
变量命名是编写可读代码的基础。只有变量被赋予一个合适的名字,才能表达它在当前环境中的意义。
命名必须传递足够的信息,形如getData
这个的函数命名就没能够提供足够的信息,这让读者无法猜测这个函数会做出些什么事情。而fetchUserInfoAsync
也许就好很多,读者至少能猜到,这个函数大约是远程获取用户信息。
- 命名的基础
通常,我们会使用 名词 来命名对象,用 动词 来命名函数。比如:
monkey.eat(banana) // the monkey eats a banana
const apple = pick(tree) // pick an apple from the tree
有时候,我们需要表示某种集合的概念,比如数组或者哈希对象。这是可以使用名词的复数形式来表示,比如bananas
表示一个数组。如果需要特别强调这种集合的形式,也可以加上 List
或 Map
后缀来表示,比如 bananaList
表示数组。 如果有些单词的复数形式和单数形式相同,比如data
, information
等,这时就用 List
为后缀表示集合概念。
- 命名的上下文
变量都是处在 上下文 (作用域)内的,变量的命名应该和上下文相契合,同一个变量,在不同的上下文,命名可以不同。
- 严格遵循一种命名规范的收益
如果你能够时刻按照某种严格的规范来命名变量和函数,还能带来一个潜在的好处,那就是你再也不用 记住 那些之前命名过或者其他人命名过的变量和函数了。比如,【获取用户信息】这个概念,就叫作fetchUserInfomation
,不管是在早晨还是晚上,家里还是在公司里,它都是命名为 fetchUserInfomation
。
- 分支结构
分支是代码里最常见的结构,一段结构清晰的代码单元,应当是像二叉树一样,呈现下面的结构。
if(condition1){
if(condition2){
...
} else {
...
}
}else {
if(condition3){
...
}else {
...
}
}
这种优美的结构能帮助我们在大脑中迅速绘制一张图,便于在脑海中模拟代码的执行。但是,我们大多数人都不会遵循上面这样的结构来写分支代码。以下是一些常见的,可读性比较差的分支语句的写法:
不好的做法:在分支中return
function foo(){
if (condition){
// 分支1的逻辑
return
}
// 分支2的逻辑
}
这种代码很常见,而且往往分支2的逻辑是先写的,也是函数的主要逻辑。这种致命的问题就是,如果读者没有注意到分支1中的 return
,就不会意识到后面一段代码(分支2)是有可能不被执行的。建议是把分支2的代码写着else
模块中,代码就会清晰可读:
function foo(){
if(condition){
// 分支1的逻辑
}else {
// 分支2的逻辑
}
}
如果某个分支为空,最好留一个空行,明确地告诉代码的读者,
如果走到这个 else ,什么也不做。这样读者就不会产生任何怀疑。
不好的做法:多个条件的复合
if(condition1 && condition2 && condition3 ){
// 分支1:做些事情
} else {
//分支2:做些事情
}
这种代码也很常见:在若干条件同时满足(或有任一满足) 的时候做一些主要事情(分支1),否则做一些次要事情(分支2)。这样笼统的使用同一段代码来处理多个分支,那么就增加了阅读者阅读分支2时的负担(需要考虑多个情况)。对于这种场景,通常这样写:
if(condition1){
if(condition2){
// 分支1: 做一些事情
}else {
// 分支2:做一些事情
}
} else {
//分支3 :做一些事情
}
即使分支2和分支3完全一样,我也认为有必要分开。不过对于那种多个复合条件联系十分紧密,就没必要分开写,比如if(foo && foo.bar)
。
不好的做法:使用分支改变环境
let foo = someValue;
if (condition){
foo = doSomethingTofoo(foo)
}
// 继续使用 foo 做一些事情
这种风格的代码很容易出现在那些屡经修改的代码文件中,很可能一开始是没有这个 if
代码块的,后来发现了一个bug,于是加上了这个if
代码块。
事实上,这样的「补丁」积累起来,很快就会摧毁代码的可读性和可维护性。怎么说呢?当我们在写下上面这段代码中的 if 分支以试图修复 bug 的时候,我们内心存在这样一个假设:我们是知道程序在执行到这一行时,foo 什么样子的;但事实是,我们根本不知道,因为在这一行之前,foo 很可能已经被另一个人所写的尝试修复另一个 bug 的另一个 if 分支所篡改了。所以,当代码出现问题的时候,我们应当完整地审视一段独立的功能代码(通常是一个函数),并且多花一点时间来修复他,比如:
const foo = condition ? doSomethingToFoo(someValue) : someValue;
我们看到,很多风险都是在项目快速迭代的过程中积累下来的。为了「快速」迭代,在添加功能代码的时候,我们有时候连函数这个最小单元的都不去了解,仅仅着眼于自己插入的那几行,希望在那几行中解决/hack掉所有问题,这是十分不可取的。
我认为,项目的迭代再快,其代码质量和可读性都应当有一个底线。这个底线是,当我们在修改代码的时候,应当完整了解当前修改的这个函数的逻辑,然后修改这个函数,以达到添加功能的目的。注意,这里的「修改一个函数」和「在函数某个位置添加几行代码」是不同的,在「修改一个函数」的时候,为了保证函数功能独立,逻辑清晰,不应该畏惧在这个函数的任意位置增删代码。
- 函数
一个函数只做一件事情
有时,我们会自作聪明的写一些很【通用】 的函数。比如,我们有可能写出下面这样一个获取用户信息的函数 fetchUserInfo
:其逻辑是:
1、当传入的参数是用户ID(字符串)时,返回单个用户的数据。
2、当传入的参数是用户ID的列表(数组)时,返回一个数组,其中每项包含一个用户的数据
sync function fetchUserInfo (id){
const isSingle = typeof id ==="string";
const idList = isSingle ? [id] : id;
const result = await request.post('/api/userInfo',{idList});
return isSingle ? result[0] :result;
}
// 可以这样调用
const userList = await fetchUserInfo(['1001','1013']);
// 也可以这样调用
const user = await fetchUserInfo('1013')
这个函数能做两件事:1)获取多个用户列表的数据;2)获取单个用户数据。这样读者在某处读到 fetchUserInfo(['1001','1013'])
这句调用代码时,会立刻对 fetchUserInfo
产生第一印象:这个函数是需要出入用户ID数组的;而当读到另一种调用方式时,就会怀疑自己之前的判断。
遵循一个函数只做一件事 的原则,我们可以将上述功能拆分成两个函数 fetchMultipleUser
和 fetchSingleUser
来实现。
async function fetchMultipleUser(idList){
return await request.post('/api/users/',{idList})
}
async function fetchSingleUser(id){
return await fetchMultipleUser([id])[0]
}
改良后的代码不仅改善了代码的可读性,也改善了可维护性,当不需要获取单一用户信息时,就可以放心大胆的直接删掉整个函数。
如何界定某个函数做的是不是一件事情?
作者的经验是:如果一个函数的参数仅仅包含输入数据(交给函数处理的数据),而没有混杂或暗含有指令(以某种约定的方式告诉函数该怎么处理数据),那么函数做的应当就是一件事情。比如说,改良前的 fetchUserInfo
函数的参数是【多个用户ID数组或单个用户的ID】,这个【或】字其实就暗含了某种指令。
函数应适当地处理异常
有时候,我们会陷入一种很不好的习惯中,那就是,总是去尝试写出永远不会报错的函数。我们会给参数配上默认值,在很多地方使用 ||
或者 &&
来避免代码运行出错,仿佛如果你的函数报错会成为某种耻辱似的。而且,当我们尝试去修复一个运行时报错的函数时,我们往往倾向于在报错的那一行添加一些兼容逻辑来避免报错。
举个例子,假设我们需要编写一个获取用户详情的函数,它要返回一个完整的用户信息对象:不仅包含ID,名字等基本信息,也包含诸如「收藏的书籍」等通过额外接口返回的信息。这些额外的接口也许不太稳定:
async function getUserDetail(id) {
const user = await fetchSingleUser(id);
user.favoriteBooks = (await fetchUserFavorits(id)).books;
// 上面这一行报错了:Can not read property 'books' of undefined.
// ...
}
假设 fetchUserFavorites 会时不时地返回 undefined,那么读取其 books 属性自然就会报错。为了修复该问题,我们很可能会这样做:
const favorites = await fetchUserFavorits(id);
user.favoriteBooks = favorites && favorites.books;
// 这下不会报错了
这样做看似解决了问题:的确,getUserDetail 不会再报错了,但同时埋下了更深的隐患。
当 fetchUserFavorites 返回 undefined 时,程序已经处于一种异常状态了,我们没有任何理由放任程序继续运行下去。试想,如果后面的某个时刻(比如用户点击「我收藏的书」选项卡),程序试图遍历 user.favoriteBooks 属性(它被赋值成了undefined),那时也会报错,而且那时排查起来会更加困难。
如何处理上述的情况呢?我认为,如果被我们依赖的 fetchUserFavorits 属于当前的项目,那么 getUserDetail 对此报错真的没什么责任,因为 fetchUserFavorits 就不应该返回 undefined,我们应该去修复 fetchUserFavorits,任务失败时显式地告知出来,或者直接抛出异常。同时,getUserDetail 稍作修改:
// 情况1:显式告知,此时应认为获取不到收藏数据不算致命的错误
const result = await fetchUserFavorits(id);
if(result.success) {
user.favoriteBooks = result.data.books;
} else {
user.favoriteBooks = []
}
// 情况2:直接抛出异常
user.favoriteBooks = (await fetchUserFavorits(id)).books;
// 这时 `getUserDetail` 不需要改动,任由异常沿着调用栈向上冒泡
那么如果 fetchUserFavorits 不在当前项目中,而是依赖的外部模块呢?我认为,这时你就该为选择了这样一个不可靠的模块负责,在 getUserDetail 中增加一些「擦屁股」代码,来避免你的项目的其他部分受到侵害。
const favorites = await fetchUserFavorits(id);
if(favorites) {
user.favoriteBooks = favorites.books;
} else {
throw new Error('获取用户收藏失败');
}
控制函数的副作用
无副作用的函数,是不依赖上下文,也不改变上下文的函数。长久依赖,我们已经习惯了去写「有副作用的函数」,毕竟 JavaScript 需要通过副作用去操作环境的 API 完成任务。这就导致了,很多原本可以用纯粹的、无副作用的函数完成任务的场合,我们也会不自觉地采取有副作用的方式。
虽然看上去有点可笑,但我们有时候就是会写出下面这样的代码!
async function getUserDetail(id) {
const user = await fetchSingleUserInfo(id);
await addFavoritesToUser(user);
...
}
async function addFavoritesToUser(user) {
const result = await fetchUserFavorits(user.id);
user.favoriteBooks = result.books;
user.favoriteSongs = result.songs;
user.isMusicFan = result.songs.length > 100;
}
上面,addFavoritesToUser 函数就是一个「有副作用」的函数,它改变了 users,给它新增了几个个字段。问题在于,仅仅阅读 getUserData 函数的代码完全无法知道,user 会发生怎样的改变。
一个无副作用的函数应该是这样的:
async function getUserDetail(id) {
const user = await fetchSingleUserInfo(id);
const {books, songs, isMusicFan} = await getUserFavorites(id);
return Object.assign(user, {books, songs, isMusicFan})
}
async function getUserFavorites(id) {
const {books, songs} = await fetchUserFavorits(user.id);
return {
books, songs, isMusicFan: result.songs.length > 100
}
}
非侵入性地改造函数
函数是一段独立和内聚的逻辑。在产品迭代的过程中,我们有时候不得不去修改函数的逻辑,为其添加一些新特性。之前我们也说过,一个函数只应做一件事,如果我们需要添加的新特性,与原先函数中的逻辑没有什么联系,那么决定是否通过改造这个函数来添加新功能,应当格外谨慎。
仍然用「向服务器查询用户数据」为例,假设我们有如下这样一个函数(为了让它看上去复杂一些,假设我们使用了一个更基本的 request 库):
const fetchUserInfo = (userId, callback) => {
const param = {
url: '/api/user',
method: 'post',
payload: {id: userId}
};
request(param, callback);
}
现在有了一个新需求:为 fetchUserInfo 函数增加一道本地缓存,如果第二次请求同一个 userId 的用户信息,就不再重新向服务器发起请求,而直接以第一次请求得到的数据返回。
按照如下快捷简单的解决方案,改造这个函数只需要五分钟时间:
const userInfoMap = {};
const fetchUserInfo = (userId, callback) => {
if (userInfoMap[userId]) { // 新增代码
callback(userInfoMap[userId]); // 新增代码
} else { // 新增代码
const param = {
// ... 参数
};
request(param, (result) => {
userInfoMap[userId] = result; // 新增代码
callback(result);
});
}
}
不知你有没有发现,经此改造,这个函数的可读性已经明显降低了。没有缓存机制前,函数很清晰,一眼就能明白,加上新增的几行代码,已经不能一眼就看明白了。
实际上,「缓存」和「获取用户数据」完全是独立的两件事。我提出的方案是,编写一个通用的缓存包装函数(类似装饰器)memorizeThunk,对 fetchUserInfo 进行包装,产出一个新的具有缓存功能的 fetchUserInfoCache,在不破坏原有函数可读性的基础上,提供缓存功能。
const memorizeThunk = (func, reducer) => {
const cache = {};
return (...args, callback) => {
const key = reducer(...args);
if (cache[key]) {
callback(...cache[key]);
} else {
func(...args, (...result) => {
cache[key] = result;
callback(...result);
});
}
}
}
const fetchUserInfo = (userInfo, callback) => {
// 原来的逻辑
}
const fetchUserInfoCache = memorize(fetchUserInfo, (userId) => userId);
也许实现这个方案需要十五分钟,但是试想一下,如果将来的某个时候,我们又不需要缓存功能了(或者需要提供一个开关来打开/关闭缓存功能),修改代码的负担是怎样的?第一种简单方案,我们需要精准(提心吊胆地)地删掉新增的若干行代码,而我提出的这种方案,是以函数为单位增删的,负担要轻很多,不是吗?
类的结构
避免滥用成员函数
总结
伟大的文学作品都是建立在废纸堆上的,不断删改作品的过程有助于写作者培养良好的「语感」。当然,代码毕竟不是艺术品,程序员没有精力也不一定有必要像作家一样反复打磨自己的代码/作品。但是,如果我们能够在编写代码时稍稍多考虑一下实现的合理性,或者在添加新功能的时候稍稍回顾一下之前的实现,我们就能够培养出一些「代码语感」。这种「代码语感」会非常有助于我们写出高质量的可读的代码。