作者介绍
原文作者: Robert C. Martin, Object Mentor公司总裁,面向对象设计、模式、UML、敏捷方法学和极限编程领域的资深顾问,是《敏捷软件开发:原则、模式、与实践》的作者。
翻译作者:韩磊,互联网产品与运营专家,技术书籍著译者。译著有《梦断代码》和《C#编程风格》等。(竟然不是程序员~~~)
内容概要
本书后几章主要讲了java相关的类、系统、和并发的设计介绍,较粗略,与简洁之道不是特别融合,故而省略,想要详细了解的建议去看更优质的详细讲解。
本书主要站在代码的可读性上讨论。可读性? 顾名思义,代码读起来简洁易懂, 让人心情愉悦,大加赞赏。
在N年以后,自己或者他人仍然能够稍加阅读就能明白其中的意思。什么是整洁代码?看看程序员鼻祖们怎么说的,
整洁代码只做好一件事。—— Bjarne Stroustrup, C++语言发明者
整洁代码从不隐藏设计者的意图。—— Grady Booch, 《面向对象分析与设计》作者
不能想着,这代码我能看懂就好了, 即使当下能看懂,那几个月甚至一年以后呢。更不能说为了体现自己编程“高大上”,故意用一些鲜为人知的语法,如
const LIMIT = 10
const LIMIT = Number(++[[]][+[]]+[+[]])
- 尽可能少的依赖关系,模块提供尽量少的API。—— Dave Thoms, OTI创始人, Eclipse战略教父
- 代码应该通过其字面表达含义,命名和内容保持一致。 —— Michael Feathers, 《修改代码的艺术》作者
- 减少重复代码,甚至没有重复代码。—— Ron Jeffries, 《C#极限编程探险》作者
- 让编程语言像是专门为解决那个问题而存在。 —— Ward Counningham, Wiki发明者
有意义的命名
- 名副其实
好的变量、函数或类的名称应该已经答复了所有的大问题,如果需要注释补充,就不算名副其实。
工具函数内部的临时变量可以稍微能接收。
// what's the meaning of the 'd'?
int d;
// what's list ?
List<int []> list; int daysSinceCreation
int daysSinceModification
此处的重定向,起名为“redirection”会不会更好一点,
/**
* 重定向
*/
public function forward(Request $request, $controller, $action) {}
/**
* 重定向
*/
public function default(Request $request, $controller, $action) {}
既是注册帐号,为何不直接命名为 register 呢?也许会说,注册就是新增帐号,create也是新增帐号,自然,create可以代表注册。可新增帐号可能是自己注册,也可能是系统分配,还可能是管理员新增帐号,业务场景不一样,实现也很可能不一样。所以,建议取一个明确的,有意义的,一语道破函数干了啥的名称。
//注册账号
public function create($data) {}
- 避免误导
程序员必须避免留下掩藏代码本意的错误线索。变量命名包含数据类型单词(array/list/num/number/str/string/obj/Object)时,需保证该变量一定是该类型,包括变量函数中可能的改变。更致命的误导是命名和内容意义不同,甚至完全相反。
// 确定是 List?
accountList = 0
// 确定是 Number?
nodeNum = '1' //确定所有情况返回值都是list吗?
function getUserList (status) {
if (!status) return false
let userList = []
...
return userList
}
.align-left {
text-align: "right";
}
- 做有意义的区分
product/productIno/productData 如何区分?哪个代表哪个意思? Info 和 Data就像 a / an / the 一样,是意义含糊的废话。如下函数命名也是没有意义的区分,
getActiveAccount()
getActiveAccounts()
getActiveAccountInfo()
- 使用读的出来的名称
读不出来就不方便记忆,不方便交流。大脑中有很大一块地方用来处理语言,不利用起来有点浪费了。
- 使用可搜索的名称
让IDE帮助自己更便捷的开发。假如在公共方法里面起个变量名叫value,全局搜索,然后一脸懵逼地盯着这上百条搜索结果。 (value vs districts)
- 每个概念对应一个词
媒体资源叫media resources 还是 publisher?
- 添加有意义的语境
firstName/lastName/street/city/state/hourseNumber
=>
addrFirstName/addrLastName/addrStreet/addrCity/addrState/hourseNumber
注释
什么也比不上放置良好的注释来的有用。
什么也不会比乱七八糟的注释更有本事搞乱一个模块。
什么也不会比陈旧、提供错误信息的注释更有破坏性。
若编程语言足够有表达力,或者我们长于用这些语言来表达意图,就不那么需要注释——也根本不需要。
- 作者为什么极力贬低注释?
注释会撒谎。由于程序员不能坚持维护注释,其存在的时间越久,离其所描述的代码越远,甚至最后可能全然错误。不准确的注释比没有注释坏的多,净瞎说,真实只在一处地方存在:代码。
- 注释的恰当用法是弥补我们在用代码表达意图时遭遇的失败。
// 禁用、解冻
public function option(Request $request) {}
// 记录操作日志
protected function writeLog($optType,$optObjectName, $optId, $optAction) {}
=>
protected function recordOperationLog($optType,$optObjectName, $optId, $optAction) {}
将上面的 注释 + 代码 合成下方纯代码,看着更简洁,且不会读不懂。
再者,可以在函数定义的地方添加说明性注释,可不能在每个用到这个函数的地方也添加注释,这样,在阅读函数调用的环境时,还得翻到定义的地方瞅瞅是什么意思。但如果函数本身的名称就能描述其意义,就不存在这个问题了。
别担心名字太长,能准确描述函数本身的意义才是更重要的。
- 注释不能美化糟糕的代码。
对于烂透的代码,最好的方法不是写点儿注释,而是把它弄干净。与其花时间整一大堆注释,不如花时间整好代码,用代码来阐述。
// check the obj can be modified
if (obj.flag || obj.status === 'EFFECTIVE' && user.info.menu === 1) {
// todo
}
if (theObjCanBeModified()) {}
function theObjCanBeModified () {}
好注释
1. 少许公司代码规范要求写的法律相关注释。
/**
* Laravel IDE Helper Generator
*
* @author Barry vd. Heuvel <barryvdh@gmail.com>
* @copyright 2014 Barry vd. Heuvel / Fruitcake Studio (http://www.fruitcakestudio.nl)
* @license http://www.opensource.org/licenses/mit-license.php MIT
* @link https://github.com/barryvdh/laravel-ide-helper
*/ namespace Barryvdh\LaravelIdeHelper;
2. 对意图的解释,如,
function testConcurrentAddWidgets() {
...
// this is our best attempt to get a race condition
// by creating large number of threads.
for (int i = 0; i < 25000; i++) {
// to handle thread
}
}
3. 阐释
有时,对于某些不能更改的标准库,使用注释把某些晦涩难懂的参数或返回值的意义翻译为某种可读的形式,也会是有用的。
function compareTest () {
// bb > ba
assertTrue(bb.compareTo(ba) === 1)
// bb = ba
assertTrue(bb.compareTo(ba) === 0)
// bb < ba
assertTrue(bb.compareTo(ba) === -1)
}
// could not find susan in students.
students.indexOf('susan') === -1
4. 警示
注释用于警告其他程序员某种后果,也是被支持的。
函数,
// Don't run unless you have some time to kill
function _testWithReallyBigFile () {}
文件顶部注释,
/**
* 文件来内容源于E:\Git_Workplace\tui\examples\views\components\color\tinyColor.js,需要新增/编辑/删除内容请更改源文件。
*/
5. TODO
来不及做的,使用TODO进行注释。虽然这个被允许存在,但不是无限书写TODO的理由,需要定期清理。
6. 放大
注释可以用来放大某些看着不合理代码的重要性。
不就是个trim()么?
// the trim is real importan. It removes the starting
// spaces that could casuse the item to be recoginized
// as another list
String listItemContent = match.group(3).trim()
没引入任何编译后的js和css,代码如何正常工作的呢?请看注释。
<body>
<div id="app"></div>
<!-- built files will be auto injected -->
</body>
7. 公共API中的DOC
公共文档的doc一般会用于自动生成API帮助文档,试想如果一个公共库没有API说明文档,得是一件多么痛苦的事儿,啃源码花费时间实在太长。
坏注释
喃喃自语
写了一些除了自己别人都看不懂的文字。-
多余的注释
简单的函数,头部位置的注释全属多余,读注释的时间比读代码的时间还长,完全没有任何实质性的作用。// Utility method that returns when this.closed is true.
// Throws an exception if the timeout is reached.
public synchronized void waitForClose(final long timeoutMillis)
throw Exception {
if (!closed)
{
wait(timeoutMillis);
if (!closed)
throw new Exception("MockResponseSender could not be closed");
}
} 误导性注释
代码为东,注释为西。多余的注释
// 创建
public function create(Request $request) {}
// 更新
public function update(Request $request) {}
// 查询
public function read(Request $request) {}
// 删除
public function delete(Request $request) {}
$table已经初始化过了,@var string 这一行注释看上去似乎就没那么必要了。
/**
* The table name for the model.
* @var string
*/
protected $table = 'order_t_creative';
5. 括号后面的注释
只要遵循函数只做一件事,尽可能地短小,就不需要如下代码所示的尾括号标记注释。
try {
...
while () {
...
} // while
...
} // try
catch () {
...
} // catch
一般不在括号后方添加注释,代码和注释不混在一行。
function handleKeydown (e) {
if (keyCode === 13) { // Enter
e.preventDefault()
if (this.focusIndex !== -1) {
this.inputValue = this.options[this.focusIndex].value
}
this.hideMenu()
}
if (keyCode === 27) { // Esc
e.preventDefault()
this.hideMenu()
}
if (keyCode === 40) { // Down
e.preventDefault()
this.navigateOptions('next')
}
if (keyCode === 38) { // Up
e.preventDefault()
this.navigateOptions('prev')
}
}
现作出如下调整,
function handleKeydown (e) {
const Enter = 13
const Esc = 27
const Down = 40
const Up = 38
e.preventDefault()
switch (keycode) {
case Enter:
if (this.focusIndex !== -1) {
this.inputValue = this.options[this.focusIndex].value
}
this.hideMenu()
break
case Esc:
this.hideMenu()
break
case Down:
this.navigateOptions('next')
break
case Up:
this.navigateOptions('prev')
break
}
}
通过定义数字变量,不仅去掉了注释,各个数字也有了自己的意义,不再是魔法数字,根据代码环境,几乎不会有人问,“27是什么意思?” 诸如此类的问题。再者,if情况过多,用switch代替,看着稍显简洁。最后,每一个都有执行了e.preventDefault()
,可以放在switch外层,进行一次书写。
6. 归属和署名
源码控制系统非常善于记住谁在何时干了什么,没有必要添加签名。新项目可以清除地知道该和谁讨论,但随着时间的推移,签名将越来越不准确。
当然,这个也见仁见智,支付宝小程序抄袭微信小程序事件的触发便是因为代码里面出现开发小哥的名字。如果为了版权需要,法律声明,我想写上作者也是没有什么大问题的。
/**
* Created by PhpStorm.
* User: XXX
* Date: 2017/9/29
* Time: 14:14
*/ namespace App\Services;
use Cache;
class CacheService implements CacheServiceInterface
{
}
/**
* 功能: 广告位管理
* User: xxx@tencent.com
* Date: 17-8-2
* Time: 下午4:47
*/
class PlacementController extends BaseController
{
}
7. 注释掉的代码
直接把代码注释掉是讨厌的做法。Don’t do that! 其他人不敢删除注释掉的代码,可能会这么想,代码依然在那儿,一定有其原因,或者这段代码很重要,不能删除。
其他人因为某些原因不敢删可以理解,但如果是自己写的注释代码,有啥不敢删呢?再重要的注释代码,删掉后,还有代码控制系统啊,这个系统会记住人为的每一次改动,还担心啥呢?放心地删吧!管它谁写的。
// $app->middleware([
// App\Http\Middleware\DemoMiddleware::class
// ]); // $app->routeMiddleware([
// 'auth' => App\Http\Middleware\Authenticate::class,
// ]); if (APP_MODE == 'dev') {
$app->register(Barryvdh\LaravelIdeHelper\IdeHelperServiceProvider::class);
}
$app->register(\App\Providers\UserServiceProvider::class);
$app->register(\App\Providers\UserRoleServiceProvider::class);
8. 信息过多
9. 别在注释中添加有趣的历史性话题或无关的细节描述。
10. 注释和代码没有明显的联系
11. 注释和代码之间的联系应该显而易见,如果注释本身还需要解释,就太糟糕了。
/**
* start with an array that is big enough to hold all the pixels
* (plus filter biytes), and extra 200 bytes for header info
*/
this.pngBytes = new byte[((this.width + 1) + this.height * 3) + 200];
12. 非公共代码的doc类注释
有些doc类的注释对公共API很有用,但如果代码不打算作公共用途,就没有必要了。
下面的四行注释,除了第一行,其它的都显得很多余,无疑在重复函数参数已经描述过的内容。倘若阅读代码的人花了时间看注释,结果啥也没有,沮丧;知道没用自动掠过,没有花时间看注释,那这注释还留着干啥。
/**
* 根据媒体ID获取广告位ID
* @param PlacementService $service
* @param Request $request
* @return Msg
*/
public function getPublisherPlacementIds(PlacementService $service, Request $request) {}
函数
- 短小
函数第一规则是要短小,第二规则是还要更短小。if语句,else语句,while语句等,其中的代码块应该只有一行。函数代码行建议不要超过20行,每行代码长度建议150个字符左右。如下代码片段,建议换行。
export default function checkPriv (store, path) {
return store.state.user.privileges && (store.state.user.privileges.includes(path) || store.state.user.privileges.includes(`/${path.split('/')[1]}/*`) || isAll(store))
}
- 函数应该只做一件事,做好这件事。
如下函数,executeSqlContent()
很明显不止做一件事, 前半部分实现了连接配置的获取,后半部分根据config执行sql。
/**
* 根据文件名和文件路径执行具体的sql
* @param $file
* @param $dbConfigPath
* @param $date
*/
protected function executeSqlContent($file, $dbConfigPath, $date)
{
$config = [];
// 获取数据库名称
if ($file == 'nn_global.sql' || $file == 'nn_pub_template.sql') {
// DB配置
$config = config("database.connections.global");
$userId = 'global'; } elseif (strpos($file, 'nn_pub') !== false) {
$fileName = explode('.', $file); $dbName = explode('_', $fileName[0]);
if (count($dbName) == 3) {
$dbInfo = UserDbTConfig::select(['onsn_name'])->where('dbn_name', $fileName[0])->first();
if ($dbInfo) {
$dbInfo = $dbInfo->toArray();
$onsInfo = zkname($dbInfo['onsn_name']);
$config = config("database.connections.individual");
// 覆盖HOST
$config['host'] = $onsInfo->ip;
$config['port'] = $onsInfo->port;
$userId = $dbName[2];
}
}
} if ($config) {
// sql语句
$dbSqlConfig = file_get_contents($dbConfigPath . $file);
if ($dbSqlConfig) {
$this->info($file . '文件内容为:' . $dbSqlConfig); // 添加新的连接
config(["database.connections.pp_pub_{$userId}" => $config]);
$db = DB::connection("nn_pub_{$userId}");
$db->statement($dbSqlConfig); // 执行成功,文件备份移动
$dirName = 'static/bak/' . $date;
if (!is_dir($dirName)) {
mkdir($dirName, 0777, true);
}
$cmd = "mv " . $dbConfigPath . $file . " " . $dirName;
shell_exec($cmd); // 断开DB连接
DB::disconnect("nn_pub_{$userId}"); $this->info($file . '文件内容为执行完成');
}
}
}
- 每个函数一个抽象层级,函数中混着不同抽象层级往往容易让人迷惑。
如下代码便是抽象层级不一样, getConnectionConfig()
,属于已经抽象过的一层函数调用,下方的文件处理却是具体的实现。
举这个例子只是为了说明不同的抽象层级是这个意思,由于函数本身不复杂,不存在让人迷惑的问题。
只是函数实现一旦混杂多了,不容易搞得清楚某一行表达式是基础概念还是细节,更多的细节就会在函数中纠结起来。
protected function executeSqlContent($file, $dbConfigPath, $date)
{
$config = $this->getConnectionConfig($file)
if ($config) {
// sql语句
$dbSqlConfig = file_get_contents($dbConfigPath . $file);
if ($dbSqlConfig) {
$this->info($file . '文件内容为:' . $dbSqlConfig);
// 添加新的连接
config(["database.connections.pp_pub_{$userId}" => $config]);
$db = DB::connection("nn_pub_{$userId}");
$db->statement($dbSqlConfig);
// 执行成功,文件备份移动
$dirName = 'static/bak/' . $date;
if (!is_dir($dirName)) {
mkdir($dirName, 0777, true);
}
$cmd = "mv " . $dbConfigPath . $file . " " . $dirName;
shell_exec($cmd);
// 断开DB连接
DB::disconnect("nn_pub_{$userId}");
$this->info($file . '文件内容为执行完成');
}
}
}
private function getConnectionConfig ($file)
{
$config = []
// 获取数据库名称
if ($file == 'nn_global.sql' || $file == 'nn_pub_template.sql') {
// DB配置
$config = config("database.connections.global");
$userId = 'global';
} elseif (strpos($file, 'nn_pub') !== false) {
$fileName = explode('.', $file);
$dbName = explode('_', $fileName[0]);
if (count($dbName) == 3) {
$dbInfo = UserDbTConfig::select(['onsn_name'])->where('dbn_name', $fileName[0])->first();
if ($dbInfo) {
$dbInfo = $dbInfo->toArray();
$onsInfo = zkname($dbInfo['onsn_name']);
$config = config("database.connections.individual");
// 覆盖HOST
$config['host'] = $onsInfo->ip;
$config['port'] = $onsInfo->port;
$userId = $dbName[2];
}
}
}
return $config
}
稍好一点的抽象层级如下,当然excuteSql()
还可以继续拆分,当书写函数的时候需要打空行来区别内容的大部分时候 可以考虑拆分函数了。
protected function executeSqlByFile($file, $dbConfigPath, $date)
{
if ($this->getConnectionConfig($file)) {
$this->excuteSql($file, $dbConfigPath, $date)
}
}
private function getConnectionConfig($file)
{
$config = []
// 获取数据库名称
if ($file == 'nn_global.sql' || $file == 'nn_pub_template.sql') {
// DB配置
$config = config("database.connections.global");
$userId = 'global';
} elseif (strpos($file, 'nn_pub') !== false) {
$fileName = explode('.', $file);
$dbName = explode('_', $fileName[0]);
if (count($dbName) == 3) {
$dbInfo = UserDbTConfig::select(['onsn_name'])->where('dbn_name', $fileName[0])->first();
if ($dbInfo) {
$dbInfo = $dbInfo->toArray();
$onsInfo = zkname($dbInfo['onsn_name']);
$config = config("database.connections.individual");
// 覆盖HOST
$config['host'] = $onsInfo->ip;
$config['port'] = $onsInfo->port;
$userId = $dbName[2];
}
}
}
return $config
}
private function excuteSql($file, $dbConfigPath, $date)
{
$dbSqlConfig = file_get_contents($dbConfigPath . $file);
if ($dbSqlConfig) {
$this->info($file . '文件内容为:' . $dbSqlConfig);
config(["database.connections.nn_pub_{$userId}" => $config]);
$db = DB::connection("nn_pub_{$userId}");
$db->statement($dbSqlConfig);
$dirName = 'static/bak/' . $date;
if (!is_dir($dirName)) {
mkdir($dirName, 0777, true);
}
$cmd = "mv " . $dbConfigPath . $file . " " . $dirName;
shell_exec($cmd);
DB::disconnect("nn_pub_{$userId}");
$this->info($file . '文件内容为执行完成');
}
}
- 使用描述性的函数名
长而具有描述性的名称,比短而令人费解的名称好。(如果短也能,当然更好)
长而具有描述性的名称,比描述性长的注释好。代码维护时,大多数程序员都会自动忽略掉注释,不能保证每次更改都实时更新,越往后越不想看注释,因为很可能形成误导,程序才是真事实。
所以,别怕长,更重要的是描述性,看到这个函数名称就知道是干啥的。读代码就像是读英文文章一样,先干了啥,后干了啥,细节怎么干的?
小窍门:可以使用IDE搜索帮助完善命名。
即使结合文件名,publisherController,打死我也无法将 all
和 移动媒体分类
联系起来。建议函数名:getMobileMediaClassification()
。
/**
* 移动媒体分类
*/
public function all(PublisherServices $service, Request $request) {}
完美命名示范,代码上方的注释或许已经不需要了,不过对于母语是中文的我们来说,就当是英文翻译了。
/**
* 根据媒体ID获取广告位ID
*/
public function getPublisherPlacementIds(PlacementService $service, Request $request)
- 函数参数
最理想的参数数量是0,其次是一,再次是二,应尽量避免三。除非有足够的理由,否则不要用三个以上的参数了。
参数多于两个,测试用例覆盖所有的可能值组合是令人生畏的。
避免出现输出参数。
- 标识参数。
向函数传入布尔参数简直就是骇人听闻的做法,这样做,就是大声宣布函数不止做一件事,为true会这样,为false会那样。非Boolean类型“标识”参数同理。
如下代码明确地指出initOrder进行了两种完全不同的初始化方式。
// 订单数据初始化分两种,一种为普通创建订单,一种为通过库存转下单
function initOrder(flag) {
if (flag === true) {
// normalInit
// ...
} else {
// init order by inventory
// ..
}
}
改进如下,也许你会说,initOrder
不还是干了两件事儿吗?不,它不是自己干了这两件事儿,它只是负责叫别人干这两件事。
如果可以的话,initOrder
里面的判断甚至可以放在能直接拿到flag
的地方。
function initOrder(flag) {
flag === true ? this.normalInit() : this.initOrderByInvenroty()
} function normalInit () {
// todo
} function initOrderByInvenroty () {
// todo
}
excuteSql($file, $dbConfigPath, $date)
中的参数 $dbConfigPath
和 $file
在file_get_contents()
的作用下变成了标识参数
,$dbSqlConfig
为真就执行主体函数,为假就不执行。
private function excuteSql($file, $dbConfigPath, $date)
{
$dbSqlConfig = file_get_contents($dbConfigPath . $file);
if ($dbSqlConfig) {
$this->info($file . '文件内容为:' . $dbSqlConfig); config(["database.connections.pp_pub_{$userId}" => $config]);
$db = DB::connection("nn_pub_{$userId}");
$db->statement($dbSqlConfig); $dirName = 'static/bak/' . $date;
if (!is_dir($dirName)) {
mkdir($dirName, 0777, true);
}
$cmd = "mv " . $dbConfigPath . $file . " " . $dirName;
shell_exec($cmd); DB::disconnect("nn_pub_{$userId}");
$this->info($file . '文件内容为执行完成');
}
}
改进如下,将标识参数拎出函数具体实现,
protected function executeSqlByFile($file, $dbConfigPath, $date)
{
if ($this->getConnectionConfig($file) && $this->file_get_contents($dbConfigPath . $file)) {
$this->excuteSql($file, $dbConfigPath, $date)
}
}
- 分隔指令与询问
函数要么做什么,要么回答什么,但二者不可兼得。函数应该修改某对象的状态或是返回该对象的相关信息,两样都干就容易导致混乱。
从读者的角度思考,set
是指是否已经设置过呢?还是设置成功呢?
if (set("username", "unclebob")) ...
也许上述代码名称可以更为 setAndCheckExists
, 但依旧没有解决实质性地问题,最好是将指令和询问分隔开来,代码如下,
if (attributeExists("username")) {
setAttribute("username", "unclebob")
}
- 使用异常替代返回错误码
错误处理代码能从主路径中分离出来,阅读的时候,可以直面主路径内容。
Promise.all([
InventoryService.read({job_id: this.jobId}),
this.getPlacementType()
]).then((result) => {
let [inventoryInfo] = result
if (res.$code !== 0) {
this.$alert.error(res.$msg)
this.$loading(false)
} else {
let ret = this.getReserveInfo(data)
if (ret.reservable) {
this.orderInitFromInventory(inventoryInfo.$data, this.defaultOrderInfo)
} else {
this.$alert.error('该库存不能下单,可能原因:库存未计算完成!')
this.$loading(false)
}
}
})
Promise.all([
InventoryService.read({job_id: this.jobId}),
this.getPlacementType()
]).then((result) => {
try {
let [inventoryInfo] = result
this.checkResponseCode(inventoryInfo)
this.isInventoryCanBeOrdered(inventoryInfo.$data)
this.orderInitFromInventory(inventoryInfo.$data, this.orderInfo)
} catch (err) {
this.$alert.error(err.message)
this.$loading(false)
}
}) isInventoryCanBeOrdered (data) {
let ret = this.getReserveInfo(data)
if (!ret.reservable) {
throw Error('该库存不能下单,可能原因:库存未计算完成!')
}
} checkResponseCode (res) {
if (res.$code !== 0) {
throw Error(res.$msg)
}
},
- 别重复自己。
重复可能是软件中一切邪恶的根源。许多原则与实践都是为控制与消除重复而创建。
created () {
this.$setServiceLoading('数据初始化中...')
let tplPromise = this.getCreativeTplList({})
let p1 = new Promise((resolve, reject) => {
publisherService.getAll({
op_status: 'ENABLE'
}).then(res => {
if (res.$code !== 0) {
reject()
} else {
this.publisherOptions = res.$data
resolve()
}
})
})
let p2 = new Promise((resolve, reject) => {
publisherService.selectAllRules().then(res => {
if (res.$code !== 0) {
reject()
} else {
this.protectionOptions = res.$data
resolve()
}
})
})
let p3 = new Promise((resolve, reject) => {
realizeService.selectAllRules().then(res => {
if (res.$code !== 0) {
reject()
} else {
this.realizeOptions = res.$data
resolve()
}
})
})
Promise.all([p1, p2, p3, tplPromise]).then(() => {
if (this.$route.query.id) {
this.isEditMode = true
placementService.read({placement_id: this.$route.query.id}).then((res) => {
if (res.$code !== 0) {
this.$alert.error(res.$msg, 3000)
} else {
Object.assign(this.formData, res.$data)
Object.keys(this.formData).forEach(key => {
if (typeof this.formData[key] === 'number') {
this.formData[key] += ''
}
})
this.$nextTick(() => {
res.$data.creative_tpl_info.forEach((tpl) => {
this.formData.tpls[this.formData.placement_type][tpl.creative_tpl_type].checked.push(tpl.creative_tpl_id)
})
this.updateCreativeIds()
})
}
}, () => {
this.$router.replace({path: '/placement/list'})
})
}
}, () => {
this.$alert.error('初始化媒体信息失败...')
this.$router.replace({path: '/placement/list'})
})
}
消除重复代码,
created () {
if (!this.$route.query || !this.$route.query.id) return
this.$setServiceLoading('数据初始化中...')
Promise.all([
publisherService.getAll({ op_status: 'ENABLE' }),
publisherService.selectAllRules({}),
realizeService.selectAllRules({}),
this.getCreativeTplList({})
]).then((resData) => {
if (!this.checkResCode(resData)) return
let [publisherOptions, protectionOptions, realizeOptions] = resData
this.publisherOptions = publisherOptions
this.protectionOptions = protectionOptions
this.realizeOptions = realizeOptions
this.isEditMode = true
placementService.read({placement_id: this.$route.query.id}).then((res) => {
if (!this.checkResCode([res])) return
Object.assign(this.formData, res.$data)
Object.keys(this.formData).forEach(key => {
if (typeof this.formData[key] === 'number') {
this.formData[key] += ''
}
})
this.$nextTick(() => {
res.$data.creative_tpl_info.forEach((tpl) => {
this.formData.tpls[this.formData.placement_type][tpl.creative_tpl_type].checked.push(tpl.creative_tpl_id)
})
this.updateCreativeIds()
})
})
})
} function checkResCode (resData) {
for (let i = 0, len = resData.length; i < len; i++) {
let res = resData[i]
if (res.$code !== 0) {
this.$alert.error(`初始化媒体信息失败,${res.$msg}`, 3000)
this.$router.replace({path: '/placement/list'})
return false
}
}
return true
}
- 别返回null,也别传递null
javascript中,需要返回值的,别返回null/undefined,也别传递null/undefined,除非特殊需要。
一旦返回值存在null,就意味着每一个调用的地方都要判断、处理null,否则就容易出现不可预料的情况。 如下方代码所示,
public void registerItem(Item item) {
if (item !== null) {
ItemRegistry registry = peristentStore.getItemRegistry();
if (registry != null) {
Item existing = registry.getItem(item.getID());
if (existing.getBillingPeriod().hasRetailOwner()) {
existing.register(item);
}
}
}
}
所以,在自己可以控制的函数中(不可控因素如:用户输入),别返回null,也别传递null,别让空判断搞乱了代码逻辑。
- 综合案例
根据《clean code》来看,下面这个函数有以下几个方面需要改进,
- 函数太大
- 代码重复
- 函数命名不具有描述性
- 部分注释位置放置不合适
- 某些行字符数量太多
//注册账号
public function create($data)
{
//检查是否可以注册
$check = [
'tdd' => $data['tdd'],
];
foreach ($check as $field => $value) {
$exist = $this->userService->check($field, $value);
if($exist) {
throw new RichException(Error::INFO_UNIQUE, [[Error::INFO_UNIQUE,['QQ']]]);
}
}
$userId = $data['user_id'];
//检查主账号是否存在
$exist = $this->userService->check('user_id', $userId);
if(!$exist) {
throw new RichException(Error::INFO_NOT_FIND_ERROR);
}
//姓名账号内唯一
$exist = (new UserModel($userId))->where('operate_name', '=', $data['operate_name'])->where('user_id', '=', $userId)->where('deleted', '=', 0)->first();
if($exist) {
throw new RichException(Error::INFO_UNIQUE, [[Error::INFO_UNIQUE,['姓名']]]);
} $time = date('Y-m-d H:i:s');
//基本信息
$exist = (new UserModel($userId))->where('tdd', '=', $data['tdd'])->where('user_id', '=', $userId)->where('deleted', '=', 1)->first();
if($exist) {
(new UserModel($userId))->where('tdd', '=', $data['tdd'])->update([
'operate_name' => $data['operate_name'],
'remarks' => isset($data['remarks']) ? $data['remarks'] : '',
'tdd' => $data['tdd'],
'time' => $time,
'operate_status' => UserModel::DEFAULT_STATUS,
'user_id' => $userId,
'deleted' => 0,
]);
} else {
(new UserModel($userId))->insert([
'operate_name' => $data['operate_name'],
'remarks' => isset($data['remarks']) ? $data['remarks'] : '',
'tdd' => $data['tdd'],
'time' => $time,
'operate_status' => UserModel::DEFAULT_STATUS,
'user_id' => $userId,
'deleted' => 0,
]);
}
//删除账号同样可以创建
$exist = (new UserQQModel())->where('tdd','=', $data['tdd'])->where('deleted', '=', 1)->first();
if($exist) {
(new UserQQModel())->where('tdd', '=', $data['tdd'])->update([
'tdd' => $data['tdd'],
'user_id' => $userId,
'user_type' => UserInfoModel::USER_TYPE_OPT,
'time' => $time,
'deleted' => 0,
]);
//删除原角色信息
(new OptUserRoleModel($userId))->where('tdd','=', $data['tdd'])->delete();
} else {
(new UserQQModel())->insert([
'tdd' => $data['tdd'],
'user_id' => $userId,
'user_type' => UserInfoModel::USER_TYPE_OPT,
'time' => $time,
'deleted' => 0,
]);
}
//角色信息
if(isset($data['role_ids']) && is_array($data['role_ids'])) {
$OptRole = array();
foreach ($data['role_ids'] as $item) {
if($item) {
$opt = [
'user_id' => $userId,
'tdd' => $data['tdd'],
'role_id' => $item,
'time' => $time,
];
$OptRole[] = $opt;
}
}
//更新角色数量信息---暂时不做维护
if($OptRole) {
(new OptUserRoleModel($userId))->insert($OptRole);
}
}
//记录日志
$operateType = BusinessLogConst::CREATE;
$operateObjectName = $data['operate_name'];
$operateId = $data['tdd'];
$operateAction = ['operate_name' => $data['operate_name'], 'remarks' => isset($data['remarks']) ? $data['remarks'] : '', 'user_id' => $userId, 'role_ids' => isset($data['role_ids']) ? json_encode($data['role_ids']) : ''];
$res = $this->writeLog($operateType, $operateObjectName, $operateId, $operateAction); return ['user_id' => $userId, 'tdd' => $data['tdd']];
}
调整后,晃一眼 registerAccount 就能知道函数干了啥,1. 能否注册判断;2. 创建帐号; 3.记录注册日志。多么完美的阅读感受。
public function registerAccount($data)
{
$this->canBeRegister($data);
$this->createAccount($data);
$this->recordRegisterLog($data);
return ['user_id' => $data['user_id'], 'tdd' => $data['tdd']];
} private function canBeRegister($data)
{
$this->isQqExist ($data);
$this->isPrimaryAccountExist($data);
$this->isUsernameUnique($data);
} private function isQqExist($data)
{
$check = [
'tdd' => $data['tdd'],
];
$exist = false
foreach ($check as $field => $value) {
$exist = $this->userService->check($field, $value);
if($exist) {
throw new RichException(Error::INFO_UNIQUE, [[Error::INFO_UNIQUE,['QQ']]]);
}
}
return $exist
} private function isPrimaryAccountExist($data)
{
$userId = $data['user_id'];
$exist = $this->userService->check('user_id', $userId);
if(!$exist) {
throw new RichException(Error::INFO_NOT_FIND_ERROR);
}
return $exist
} private function isUsernameUnique($data)
{
$userId = $data['user_id'];
$exist = (new UserModel($userId))->where('operate_name', '=', $data['operate_name'])->where('user_id', '=', $userId)->where('deleted', '=', 0)->first();
if($exist) {
throw new RichException(Error::INFO_UNIQUE, [[Error::INFO_UNIQUE,['姓名']]]);
}
return $exist
} private function createAccount($data)
{
$this->createAccounInUserModel($data)
$this->createAccountInUserQqModel($data)
$this->updateRoleInfo($data)
} private function createAccounInUserModel($data)
{
$userInfo = [
'operate_name' => $data['operate_name'],
'remarks' => isset($data['remarks']) ? $data['remarks'] : '',
'tdd' => $data['tdd'],
'time' => $time,
'operate_status' => UserModel::DEFAULT_STATUS,
'user_id' => $userId,
'deleted' => 0,
]
$exist = (new UserModel($userId))->where('tdd', '=', $data['tdd'])->where('user_id', '=', $userId)->where('deleted', '=', 1)->first();
if($exist) {
(new UserModel($userId))->where('tdd', '=', $data['tdd'])->update($userInfo);
} else {
(new UserModel($userId))->insert($userInfo);
}
} private function createAccountInUserQqModel($data)
{
$userInfo = [
'tdd' => $data['tdd'],
'user_id' => $userId,
'user_type' => UserInfoModel::USER_TYPE_OPT,
'time' => $time,
'deleted' => 0,
]
$exist = (new UserQQModel())->where('tdd','=', $data['tdd'])->where('deleted', '=', 1)->first();
if($exist) {
(new UserQQModel())->where('tdd', '=', $data['tdd'])->update($userInfo);
(new OptUserRoleModel($userId))->where('tdd','=', $data['tdd'])->delete();
} else {
(new UserQQModel())->insert($userInfo);
}
} private function updateRoleInfo($data)
{
if(isset($data['role_ids']) && is_array($data['role_ids'])) {
$OptRole = array();
foreach ($data['role_ids'] as $item) {
if($item) {
$opt = [
'user_id' => $userId,
'tdd' => $data['tdd'],
'role_id' => $item,
'time' => $time,
];
$OptRole[] = $opt;
}
}
//更新角色数量信息---暂时不做维护
if($OptRole) {
(new OptUserRoleModel($userId))->insert($OptRole);
}
}
} private function recordRegisterLog($data)
{
$operateType = BusinessLogConst::CREATE;
$operateObjectName = $data['operate_name'];
$operateId = $data['tdd'];
$operateAction = ['operate_name' => $data['operate_name'], 'remarks' => isset($data['remarks']) ? $data['remarks'] : '', 'user_id' => $userId, 'role_ids' => isset($data['role_ids']) ? json_encode($data['role_ids']) : ''];
$res = $this->writeLog($operateType, $operateObjectName, $operateId, $operateAction);
}
没有人能一开始就写出完美的程序,完美的系统,都是通过一点点改进达到一个更好的状态。
格式
今天编写的功能,极有可能在下一版本中被修改,但代码的可读性却会以后可能发生的修改行为产生深远影响。即使代码可能已经不存在,但历史风格及其律条扔能存活下来。
垂直格式
- 向报纸学习
写的好的报纸文章,从上往下阅读,在顶部,期望有个条目,告诉我们故事主题,以便让我们决定是否要继续读下去。第一段是整个故事的大纲,给出粗线条概述,但隐藏了故事细节,接着读,细节渐次增加,知道你了解所有的日期、名字、引语、说法及其他细节。
源文件也要像报纸文章那样,名称应当简单且一目了然。源文件顶部给出高层次概念和算法,细节往下渐次展开,直到找到源文件中最底层的函数和细节。
- 垂直方向上的区隔
几乎所有的代码都是从上往下读,从左往右读。每行展现一个表达式或一个句子,每组代码航展示一条完整的思路。这些思路用空白行区隔开来。
根据如下两段,可以看看添加空白行和不添加空白行的阅读体验,
package fitnesse.wikitext.widgets;
import java.util.regex.*;
public class BoldWidget extends ParentWidget {
public static final String REGEXP = "'''.+?'''";
private static final Pattern pattern = Pattern.compile("'''.+?'''",
Patter.MULTILINE + Pattern.DOTALL
);
public BoldWidget(ParentWidget parent, string text) throw Exception {
super(parent);
Matcher match = pattern.matcher(text);
match.find();
addChildWidgets(match.group(1));
}
public String render() throws Exception {
StringBuffer html = new StringBuffer("<b>");
html.append(childHtml).append("</b>");
return html.toString()
}
}
阅读代码一般是一部分一部分地理解,然,看见上面那么大一部分,第一感受,就不是很友好,第一眼便有种预感要花很多时间理解,放弃的情绪很容易就此而生。且再看看下面这段代码,
package fitnesse.wikitext.widgets; import java.util.regex.*; public class BoldWidget extends ParentWidget {
public static final String REGEXP = "'''.+?'''";
private static final Pattern pattern = Pattern.compile("'''.+?'''",
Patter.MULTILINE + Pattern.DOTALL
); public BoldWidget(ParentWidget parent, string text) throw Exception {
super(parent);
Matcher match = pattern.matcher(text);
match.find();
addChildWidgets(match.group(1));
} public String render() throws Exception {
StringBuffer html = new StringBuffer("<b>");
html.append(childHtml).append("</b>");
return html.toString()
}
}
- 垂直方向的靠近
如果说空白行隔离了概念,那么代码行的靠近便暗示了它们之间的紧密联系。所以,紧密相关的代码应该互相靠近。
曾经的苦恼:在某个类中搜索,从一个函数调到另一个函数,上下求索,想要弄清函数之间如何协作,最后却被搞的摸不清头脑,真是伤神。而想要了解系统做什么,就需要花时间和经历记住那些代码碎片在哪里。
所以,建议关系密切的概念互相靠近,虽然这条规则不适用分布在不同文件中的概念,但在设计书写时尽可能地别把关系密切的概念放在不同的文件中。
- 变声声明应尽可能靠近其使用的位置,函数很短,本地变量应该在函数的顶部出现
- 实体变量应该在类的顶部声明
- 相关函数放在一起,调用者尽可能放在被调用者上面
- 概念相关的代码应该放在一起,相关性越强,彼此之间的距离就该越短
- 垂直顺序
阅读一般是从上往下的顺序,所以,编写的时候也应该自上而下展示函数调用一来顺序。也就是说,被调用函数应该放在执行调用的函数下面。
横向格式
- 水平方向的区隔和靠近
因运算符有两个确定而重要的的要素,左边和右边,其两边的空格加强了分隔效果,而函数后面的括号没有加空格也是为了强调其紧密关系。
private void measureLine(String line) {
lineCount++;
int lineSize = line.length()
totalChars += lineSize;
lineWidthHistogram.addLine(lineSize, lineCount);
recordWidestLinet(lineSize)
}
- 水平对齐
如下代码所示的对齐方式,像是在强调不重要的东西,把阅读者的目光从真正意义上拉开。
每每阅读到此处,总会情不自禁的纵向阅读,然,纵向的数据并没有什么关联,我们更需要了解知道的是,横向的数据之间是怎样的结果。再者,格式化工具还会自动消除这类对齐。
所以,尽量不要为了表面上的美观影响到真实的阅读感受,甚至处理格式化的不一致。
public function validator(Request $request)
{
$this->validate($request, [
'user_id' => 'required|integer|min:1',
'page_size' => 'integer|between:1,100',
'page' => 'integer|min:1',
'filter' => [
'sometimes',
'filterJson' => [
['field' => 'creative_name', 'operator' => ['EQUALS', 'CONTAINS'], 'value' => 'string'],
['field' => 'ad_type_id', 'operator' => ['EQUALS'], 'value' => 'integer']
]
],
'order_by' => [
'sometimes',
'orderByJson' => [
'sort_field' => 'required|in:add_time',
'sort_order' => 'required|in:ASC,DESC'
]
],
]);
}
无对齐写法,
public function validator(Request $request)
{
$this->validate($request, [
'user_id' => 'required|integer|min:1',
'page_size' => 'integer|between:1,100',
'page' => 'integer|min:1',
'filter' => [
'sometimes',
'filterJson' => [
['field' => 'creative_name', 'operator' => ['EQUALS', 'CONTAINS'], 'value' => 'string'],
['field' => 'ad_type_id', 'operator' => ['EQUALS'], 'value' => 'integer']
]
],
'order_by' => [
'sometimes',
'orderByJson' => [
'sort_field' => 'required|in:add_time',
'sort_order' => 'required|in:ASC,DESC'
]
],
]);
}
团队规则
每个程序员都有自己喜欢的格式规则,但如果在一个团队中工作,就是团队说了算。
既是团队,一组开发者应当认同一种格式风格,每个人都采用那种风格。别让阅读的人觉得项目代码是由一大票意见无法统一的程序员写的。
对象数据结构
将私有变量设为private有一个理由,我们不想其他人依赖这些变量,任意操控这些变量,可在有些程序里,却出现了各种各样的取值器和赋值器,私有变量被公之于众,最后如同本身就是共有变量一般。
根据得墨忒耳律,模块不应该了解它所操作的对象(或类)的内部情况,根据暴露出的方法完成业务需求,如果实在需要改变,可以选择调整类或继承类。
单元测试
TDD三定律,
- 在编写不能通过的单元测试前,不可能编写生产代码。
- 只可编写刚好无法通过的单元测试,不能编译也算不通过。
- 只可编写刚好足以通过当前失败测试的生产代码。
测试代码也会越来越多,定不要不看重测试代码,要像生产代码一样对待,否则当测试代码混乱不堪的时候,前面所做的一切都白费了,没有人愿意再去管理这庞大而混乱的测试代码。所以,测试代码和生产代码一样重要,不是二等公民,需要被思考、被设计和被照料,像生产代码一样保持整洁。
整洁的测试需要遵循的五条规则 F.I.R.S.T,
快速(Fast),测试运行速度需要够快。运行慢了,就不愿意经常运行测试,就不能及时发现问题。
独立(Independent),测试应该相互独立。相互影响的测试,可能因为上一级测试不通过导致后面一连串的不通过。
可重复(Repeatable),测试应当在任何环境中重复通过。 Not repeat code, but repeat environment.
自足验证(Sele-Validating),测试应该有布尔值输出。无论通过或失败,不应该通过查看日志来确认测试是否通过。有了现代单元测试工具,此项无需担心。
及时(Timely),测试应及时编写。即单元测试刚巧在使其通过的生产代码之前编写,因为生产代码之后编写很容易造成生成代码难以测试,不知道咋写。