作者介紹
原文作者: 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']];
}
