| 《Clean Code》 代码简洁之道 | 您所在的位置：网站首页 › cleancode工具 › 《Clean Code》 代码简洁之道 |
|
作者介绍
原文作者： 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 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 * @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，代码如何正常工作的呢？请看注释。
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: [email protected] * 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']]; }
|
| CopyRight 2018-2019 实验室设备网 版权所有 |