本文羅列JavaScript程式碼中常見的程式碼壞味道，如臨時定時器，雙向資料繫結的坑，複雜的分支語句，重複賦值等，對它們進行分析如現場還原，糟糕程式碼回顧，問題診斷和識別（通過ESlint或其他工具），程式碼重構方案，給出了怎麼寫好程式碼的一手經驗~
繞來繞去，很燒腦
問題現場：
如果單詞以子音開頭（或子音集），把它剩餘的步伐移到前面，並且新增上『ay』如pig -> igpay
如果單詞以母音開頭，保持順序但是在結尾加上『way』如，egg->eggway等
糟糕程式碼：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 |
/* const */ var CONSONANTS = 'bcdfghjklmnpqrstvwxyz'; /* const */ var VOWELS = 'aeiou'; function englishToPigLatin(english) { /* const */ var SYLLABLE = 'ay'; var pigLatin = ''; if (english !== null & english.length > 0 && (VOWELS.indexOf(english[0]) > -1 || CONSONANTS.indexOf(english[0]) > -1 )) { if (VOWELS.indexOf(english[0]) > -1) { pigLatin = english + SYLLABLE; } else { var preConsonants = ''; for (var i = 0; i english.length; ++i) { if (CONSONANTS.indexOf(english[i]) > -1) { preConsonants += english[i]; if (preConsonants == 'q' & i+1 english.length && english[i+1] == 'u') { preConsonants += 'u'; i += 2; break; } } else { break; } } pigLatin = english.substring(i) + preConsonants + SYLLABLE; } } return pigLatin; } |
問題在哪：
- 太多語句
- 太多巢狀
- 太高複雜度
檢測出問題：
關於Lint的配置項：如最大語句數，複雜度，最大巢狀數，最大長度，最多傳參，最多巢狀回撥
|
1 2 |
/*jshint maxstatements:15, maxdepth:2, maxcomplexity:5 */ /*eslint max-statements:[2, 15], max-depth:[1, 2], complexity:[2, 5] */ |
|
1 2 3 |
7:0 - Function 'englishToPigLatin' has a complexity of 7. 7:0 - This function has too many statements (16). Maximum allowed is 15. 22:10 - Blocks are nested too deeply (5). |
測試先行：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 |
describe('Pig Latin', function() { describe('Invalid', function() { it('should return blank if passed null', function() { expect(englishToPigLatin(null)).toBe(''); }); it('should return blank if passed blank', function() { expect(englishToPigLatin('')).toBe(''); }); it('should return blank if passed number', function() { expect(englishToPigLatin('1234567890')).toBe(''); }); it('should return blank if passed symbol', function() { expect(englishToPigLatin('~!@#$%^&*()_+')).toBe(''); }); }); describe('Consonants', function() { it('should return eastbay from beast', function() { expect(englishToPigLatin('beast')).toBe('eastbay'); }); it('should return estionquay from question', function() { expect(englishToPigLatin('question')).toBe('estionquay'); }); it('should return eethray from three', function() { expect(englishToPigLatin('three')).toBe('eethray'); }); }); describe('Vowels', function() { it('should return appleay from apple', function() { expect(englishToPigLatin('apple')).toBe('appleay'); }); }); }); |
重構後程式碼：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 |
const CONSONANTS = ['th', 'qu', 'b', 'c', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'p', 'q', 'r', 's', 't', 'v', 'w', 'x', 'y', 'z']; const VOWELS = ['a', 'e', 'i', 'o', 'u']; const ENDING = 'ay'; let isValid = word => startsWithVowel(word) || startsWithConsonant(word); let startsWithVowel = word => !!~VOWELS.indexOf(word[0]); let startsWithConsonant = word => !!~CONSONANTS.indexOf(word[0]); let getConsonants = word => CONSONANTS.reduce((memo, char) => { if (word.startsWith(char)) { memo += char; word = word.substr(char.length); } return memo; }, ''); function englishToPigLatin(english='') { if (isValid(english)) { if (startsWithVowel(english)) { english += ENDING; } else { let letters = getConsonants(english); english = `${english.substr(letters.length)}${letters}${ENDING}`; } } return english; } |
資料對比：
max-statements: 16 → 6
max-depth: 5 → 2
complexity: 7 → 3
max-len: 65 → 73
max-params: 1 → 2
max-nested-callbacks: 0 → 1
相關資源：
jshint – http://jshint.com/
eslint – http://eslint.org/
jscomplexity – http://jscomplexity.org/
escomplex – https://github.com/philbooth/escomplex
jasmine – http://jasmine.github.io/
貼上複製
問題現場：
我們需要實現如下的效果

糟糕的程式碼：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
var boxes = document.querySelectorAll('.Box'); [].forEach.call(boxes, function(element, index) { element.innerText = "Box: " + index; element.style.backgroundColor = '#' + (Math.random() * 0xFFFFFF 0).toString(16); }); var circles = document.querySelectorAll(".Circle"); [].forEach.call(circles, function(element, index) { element.innerText = "Circle: " + index; element.style.color = '#' + (Math.random() * 0xFFFFFF 0).toString(16); }); |
問題出在哪：
因為我們在貼上複製！！
檢測出問題：
檢查出貼上複製和結構類似的程式碼片段 – jsinspect
https://github.com/danielstjules

從你的JS，TypeScript，C#，Ruby，CSS，HTML等原始碼中找到貼上複製的部分 – JSCPD
https://github.com/kucherenko/jscpd

重構後程式碼：
- Let’s pull out the random color portion…
- Let’s pull out the weird [].forEach.call portion…
- Let’s try to go further…
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 |
let randomColor = () => `#${(Math.random() * 0xFFFFFF 0).toString(16)}; let $$ = selector => [].slice.call(document.querySelectorAll(selector || '*')); let updateElement = (selector, textPrefix, styleProperty) => { $$(selector).forEach((element, index) => { element.innerText = textPrefix + ': ' + index; element.style[styleProperty] = randomColor(); }); } updateElement('.Box', 'Box', 'backgroundColor'); // 12: Refactored updateElement('.Circle', 'Circle', 'color'); // 14: Refactored |
複雜的分支語句
糟糕的程式碼：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 |
function getArea(shape, options) { var area = 0; switch (shape) { case 'Triangle': area = .5 * options.width * options.height; break; case 'Square': area = Math.pow(options.width, 2); break; case 'Rectangle': area = options.width * options.height; break; default: throw new Error('Invalid shape: ' + shape); } return area; } getArea('Triangle', { width: 100, height: 100 }); getArea('Square', { width: 100 }); getArea('Rectangle', { width: 100, height: 100 }); getArea('Bogus'); |
問題出在哪：
違反了 open/close 原則：
軟體元素（類，模組和方法等）應該易於被開啟擴充套件，但是除了本身不要多於的修改。既程式碼本身可以允許它的行為被擴充套件，但是不要修改原始碼
可以使用諸如檢查：
no-switch – disallow the use of the switch statement
no-complex-switch-case – disallow use of complex switch statements
重構後程式碼：
這時候新增一個程式碼就不像之前那樣該原先的switch，直到它又長又臭，還容易把之前的程式碼邏輯broken掉。
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 |
(function(shapes) { // triangle.js var Triangle = shapes.Triangle = function(options) { this.width = options.width; this.height = options.height; }; Triangle.prototype.getArea = function() { return 0.5 * this.width * this.height; }; }(window.shapes = window.shapes || {})); function getArea(shape, options) { var Shape = window.shapes[shape], area = 0; if (Shape & typeof Shape === 'function') { area = new Shape(options).getArea(); } else { throw new Error('Invalid shape: ' + shape); } return area; } getArea('Triangle', { width: 100, height: 100 }); getArea('Square', { width: 100 }); getArea('Rectangle', { width: 100, height: 100 }); getArea('Bogus'); // circle.js (function(shapes) { var Circle = shapes.Circle = function(options) { this.radius = options.radius; }; Circle.prototype.getArea = function() { return Math.PI * Math.pow(this.radius, 2); }; Circle.prototype.getCircumference = function() { return 2 * Math.PI * this.radius; }; }(window.shapes = window.shapes || {})); |
魔法數字/字串的壞味道
糟糕程式碼：
如上面看到的，如Magic Strings，對於諸如Triangle，Square這些就是特殊字串。
問題出在哪：
這些魔法數字和字串是直接寫死在程式碼中，不容易修改和閱讀。注入password.length > 9，這裡面的9是指 MAX_PASSWORD_SIZE ，這樣先定義後使用更清晰。同時如果多個地方需要這個判斷規則，也可以避免多次修改類似9這樣的數字
https://en.wikipedia.org/wiki/Magic_number_(programming)
http://stackoverflow.com/questions/47882/what-is-a-magic-number-and-why-is-it-bad
重構後程式碼：
1 通過物件
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
var shapeType = { triangle: 'Triangle' // 2: Object Type }; function getArea(shape, options) { var area = 0; switch (shape) { case shapeType.triangle: // 8: Object Type area = .5 * options.width * options.height; break; } return area; } getArea(shapeType.triangle, { width: 100, height: 100 }); // 15: |
2 通過 const 和 symbols
|
1 2 3 4 5 6 7 8 |
const shapeType = { triangle: Symbol() // 2: Enum-ish }; function getArea(shape, options) { var area = 0; switch (shape) { case shapeType.triangle: // 8: Enum-ish |
程式碼深淵
糟糕程式碼：
|
1 2 3 4 5 6 7 8 9 |
Person.prototype.brush = function() { var that = this; this.teeth.forEach(function(tooth) { that.clean(tooth); }); console.log('brushed'); }; |
問題出在哪：
奇奇怪怪的 self /that/_this 等
使用一下的eslint：
- no-this-assign (eslint-plugin-smells)
- consistent-this
- no-extra-bind
重構後程式碼：
利用Function.bind, 2nd parameter of forEach, es6
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 |
Person.prototype.brush = function() { this.teeth.forEach(function(tooth) { this.clean(tooth); }.bind(this)); // 4: Use .bind() to change context console.log('brushed'); }; Person.prototype.brush = function() { this.teeth.forEach(function(tooth) { this.clean(tooth); }, this); // 4: Use 2nd parameter of .forEach to change context console.log('brushed'); }; Person.prototype.brush = function() { this.teeth.forEach(tooth => { // 2: Use ES6 Arrow Function to bind `this` this.clean(tooth); }); console.log('brushed'); }; |
脆裂的字元拼接
糟糕程式碼：
|
1 2 3 4 |
var build = function(id, href, text) { return $( "<div id='tab'><a href='" + href + "' id='" + id + "'>" + text + "</a></div>" ); } |
問題出在哪：
程式碼很醜陋，也很囉嗦，不直觀。
使用 ES6的模板字串（字串插值和多行）
很多工具和框架也都提供了響應的支援，如lodash/underscore，angular，react 等
- no-complex-string-concat
重構後程式碼：
|
1 2 3 4 5 6 |
var build = (id, href, text) => `<div id="tab"><a href="${href}" id="${id}">${text}</a></div>`; var build = (id, href, text) => `<div id="tab"> <a href="${href}" id="${id}">${text}</a> </div>`; |
jQuery詢問
糟糕程式碼：
|
1 2 3 4 5 6 7 8 9 10 |
$(document).ready(function() { $('.Component') .find('button') .addClass('Component-button--action') .click(function() { alert('HEY!'); }) .end() .mouseenter(function() { $(this).addClass('Component--over'); }) .mouseleave(function() { $(this).removeClass('Component--over'); }) .addClass('initialized'); }); |
問題出在哪：
太多的鏈式呼叫
重構後程式碼：
|
1 2 3 4 5 6 7 8 9 10 |
// Event Delegation before DOM Ready $(document).on('mouseenter mouseleave', '.Component', function(e) { $(this).toggleClass('Component--over', e.type === 'mouseenter'); }); $(document).on('click', '.Component button', function(e) { alert('HEY!'); }); $(document).ready(function() { $('.Component button').addClass('Component-button--action'); }); |
臨時定時器
糟糕程式碼：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
setInterval(function() { console.log('start setInterval'); someLongProcess(getRandomInt(2000, 4000)); }, 3000); function someLongProcess(duration) { setTimeout( function() { console.log('long process: ' + duration); }, duration ); } function getRandomInt(min, max) { return Math.floor(Math.random() * (max - min + 1)) + min; } |
問題出在哪：
out of sync timer 不能確認時序和執行
重構後程式碼：
等 3s 去執行timer(setTimeout)，然後呼叫 someLongProcess (long process: random time )，接著在迴圈
使用setInterval fn（其中在進行 long process），還是通過callback傳遞來反覆setTimeout(timer, )
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
setTimeout(function timer() { console.log('start setTimeout') someLongProcess(getRandomInt(2000, 4000), function() { setTimeout(timer, 3000); }); }, 3000); function someLongProcess(duration, callback) { setTimeout(function() { console.log('long process: ' + duration); callback(); }, duration); } /* getRandomInt(min, max) {} */ |
重複賦值
糟糕程式碼：
|
1 2 3 4 |
data = this.appendAnalyticsData(data); data = this.appendSubmissionData(data); data = this.appendAdditionalInputs(data); data = this.pruneObject(data); |
問題出在哪：
有些重複和囉嗦
eslint-plugin-smells
- no-reassign
重構後程式碼：
1 巢狀的函式呼叫
2 forEach
3 reduce
4 flow
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 |
// 1 data = this.pruneObject( this.appendAdditionalInputs( this.appendSubmissionData( this.appendAnalyticsData(data) ) ) ); // 2 var funcs = [ this.appendAnalyticsData, this.appendSubmissionData, this.appendAdditionalInputs, this.pruneObject ]; funcs.forEach(function(func) { data = func(data); }); // 3 // funcs 定義如上 data = funcs.reduce(function(memo, func) { return func(memo); }, data); // 4 data = _.flow( this.appendAnalyticsData, this.appendSubmissionData, this.appendAdditionalInputs, this.pruneObject )(data); |
不合理的情報
糟糕程式碼：
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 |
function ShoppingCart() { this.items = []; } ShoppingCart.prototype.addItem = function(item) { this.items.push(item); }; function Product(name) { this.name = name; } Product.prototype.addToCart = function() { shoppingCart.addItem(this); }; var shoppingCart = new ShoppingCart(); var product = new Product('Socks'); product.addToCart(); console.log(shoppingCart.items); |
問題出在哪：
依賴被緊緊的耦合了
相互呼叫，耦合！如 product 和 shoppingCart 關係
重構後程式碼：
1 dependency injection 依賴注入
2 訊息經紀人broker
|
1 2 3 4 5 6 7 8 9 10 11 12 |
function Product(name, shoppingCart) { // 6: Accept Dependency this.name = name; this.shoppingCart = shoppingCart; // 8: Save off Dependency } Product.prototype.addToCart = function() { this.shoppingCart.addItem(this); }; var shoppingCart = new ShoppingCart(); var product = new Product('Socks', shoppingCart); // 15: Pass in Dependency product.addToCart(); console.log(shoppingCart.items); |
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
var channel = postal.channel(); // 1: Broker function ShoppingCart() { this.items = []; channel.subscribe('shoppingcart.add', this.addItem); // 5: Listen to Message } ShoppingCart.prototype.addItem = function(item) { this.items.push(item); }; function Product(name) { this.name = name; } Product.prototype.addToCart = function() { channel.publish('shoppingcart.add', this); // 13: Publish Message }; var shoppingCart = new ShoppingCart(); var product = new Product('Socks'); product.addToCart(); console.log(shoppingCart.items); |
不斷的互動呼叫
糟糕程式碼：
|
1 2 3 4 5 6 7 |
var search = document.querySelector('.Autocomplete'); search.addEventListener('input', function(e) { // Make Ajax call for autocomplete console.log(e.target.value); }); |
問題出在哪：
會造成卡頓，多餘的計算等
重構後程式碼：
throttle 和 debounce
|
1 2 3 4 5 6 7 8 9 10 11 |
var search = document.querySelector('.Autocomplete'); search.addEventListener('input', _.throttle(function(e) { // Make Ajax call for autocomplete console.log(e.target.value); }, 500)); var search = document.querySelector('.Autocomplete'); search.addEventListener('input', _.debounce(function(e) { // Make Ajax call for autocomplete console.log(e.target.value); }, 500)); |
匿名演算法
糟糕程式碼：
|
1 2 3 4 5 6 7 |
var search = document.querySelector('.Autocomplete'); search.addEventListener('input', _.debounce(function(e) { // Make Ajax call for autocomplete console.log(e.target.value); }, 500)); |
問題出在哪：
匿名函式是個好東西，但是給函式命名可以幫助我們：
- Stack Trace（結合Devtools，跟容易debug）
- Dereferencing
- Code Reuse
重構後程式碼：
|
1 2 3 4 5 |
var search = document.querySelector('.Autocomplete'); search.addEventListener('input', _.debounce(function matches(e) { console.log(e.target.value); }, 500)); |
未明確的執行
糟糕程式碼：
明確觸發時機而不是，寫在 domready
|
1 2 3 4 5 6 7 |
$(document).ready(function() { // wire up event handlers // declare all the things // etc... }); |
問題出在哪：
很難做單元測試
重構後程式碼：
利用單例模組，加上構建器函式
單例模式(單例有個init 方法，來Kick off) & 建構函式(new Application() -> 在原來的建構函式中Kick off your code!)
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 |
(function(myApp) { myApp.init = function() { // kick off your code }; myApp.handleClick = function() {}; // etc... }(window.myApp = window.myApp || {})); // Only include at end of main application... $(document).ready(function() { window.myApp.init(); }); var Application = (function() { function Application() { // kick off your code } Application.prototype.handleClick = function() {}; return Application; }()); // Only include at end of main application... $(document).ready(function() { new Application(); }); |
雙向資料繫結
糟糕程式碼：
隨便看看你們手頭的MVVM專案（如Angular的等）
問題出在哪：
很難定位執行順序和資料流（Hard to track execution & data flow ）
（也是Angular1.x被大力吐槽的地方，被React的Flux）
重構後程式碼：
方案: flux(action, dispatcher, store->view)
React Flux
An Angular2 Todo App: First look at App Development in Angular2
結語
更多的lint規則，在npm上搜尋 eslint-plugin 查詢