怎么消除JavaScript中的代码坏味道

发表于2016-04-16
评论1 681浏览

想免费获取内部独家PPT资料库?观看行业大牛直播?点击加入腾讯游戏学院游戏程序行业精英群

711501594
  本文罗列JavaScript代码中常见的代码坏味道,如临时定时器,双向数据绑定的坑,复杂的分支语句,重复赋值等,对它们进行分析如现场还原,糟糕代码回顾,问题诊断和识别(通过ESlint或其他工具),代码重构方案,给出了怎么写好代码的一手经验~

一、绕来绕去,很烧脑
1、问题现场:
  如果单词以辅音开头(或辅音集),把它剩余的步伐移到前面,并且添加上『ay』如pig -> igpay
  如果单词以元音开头,保持顺序但是在结尾加上『way』如,egg->eggway等
2、糟糕代码:
/* 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;
}
3、问题在哪:
 · 太多语句
 · 太多嵌套
 · 太高复杂度
4、检测出问题:
  关于Lint的配置项:如最大语句数,复杂度,最大嵌套数,最大长度,最多传参,最多嵌套回调
/*jshint maxstatements:15, maxdepth:2, maxcomplexity:5 */
/*eslint max-statements:[2, 15], max-depth:[1, 2], complexity:[2, 5] */
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).
5、测试先行:
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');
    });
  });
});
6、重构后代码:
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;
}
7、数据对比:
max-statements: 16 → 6
max-depth: 5 → 2
complexity: 7 → 3
max-len: 65 → 73
max-params: 1 → 2
max-nested-callbacks: 0 → 1
8、相关资源:
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、糟糕的代码:
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);
});
var circles = document.querySelectorAll(".Circle");
[].forEach.call(circles, function(element, index) {
  element.innerText = "Circle: " + index;
  element.style.color =
    '#' + (Math.random() * 0xFFFFFF  0).toString(16);
});
3、问题出在哪:
  因为我们在粘贴复制!!
4、检测出问题:
  检查出粘贴复制和结构类似的代码片段 – jsinspect
  https://github.com/danielstjules


  从你的JS,TypeScript,C#,Ruby,CSS,HTML等源代码中找到粘贴复制的部分 – JSCPD
  https://github.com/kucherenko/jscpd


5、重构后代码:
Let’s pull out the random color portion…
Let’s pull out the weird [].forEach.call portion…
Let’s try to go further…
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、糟糕的代码:
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');
2、问题出在哪:
  违反了 open/close 原则:
  软件元素(类,模块和方法等)应该易于被打开扩展,但是除了本身不要多于的修改。既代码本身可以允许它的行为被扩展,但是不要修改源代码
  可以使用诸如检查:
  no-switch – disallow the use of the switch statement
  no-complex-switch-case – disallow use of complex switch statements
3、重构后代码:
  这时候添加一个代码就不像之前那样该原先的switch,直到它又长又臭,还容易把之前的代码逻辑broken掉。
(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 || {}));

四、魔法数字/字符串的坏味道
1、糟糕代码:
  如上面看到的,如Magic Strings,对于诸如Triangle,Square这些就是特殊字符串。
2、问题出在哪:
  这些魔法数字和字符串是直接写死在代码中,不容易修改和阅读。注入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
3、重构后代码:
  通过对象
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:
  通过 const 和 symbols
const shapeType = {
  triangle: Symbol() // 2: Enum-ish
};
function getArea(shape, options) {
  var area = 0;
  switch (shape) {
    case shapeType.triangle: // 8: Enum-ish

五、代码深渊
1、糟糕代码:
Person.prototype.brush = function() {
  var that = this;
  this.teeth.forEach(function(tooth) {
    that.clean(tooth);
  });
  console.log('brushed');
};
2、问题出在哪:
  奇奇怪怪的 self /that/_this 等
  使用一下的eslint:
  no-this-assign (eslint-plugin-smells)
  consistent-this
  no-extra-bind
3、重构后代码:
  利用Function.bind, 2nd parameter of forEach, es6
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、糟糕代码:
var build = function(id, href, text) {
  return $( "" );
}
2、问题出在哪:
  代码很丑陋,也很啰嗦,不直观。
  使用 ES6的模板字符串(字符串插值和多行)
  很多工具和框架也都提供了响应的支持,如lodash/underscore,angular,react 等
  no-complex-string-concat
3、重构后代码:
var build = (id, href, text) =>
  ``;
var build = (id, href, text) => `
`;

七、jQuery询问
1、糟糕代码:
$(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');
});
2、问题出在哪:
  太多的链式调用
3、重构后代码:
// 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、糟糕代码:
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;
}
2、问题出在哪:
  out of sync timer 不能确认时序和执行
3、重构后代码:
  等 3s 去执行timer(setTimeout),然后调用 someLongProcess (long process: random time ),接着在循环
使用setInterval fn(其中在进行 long process),还是通过callback传递来反复setTimeout(timer, )
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、糟糕代码:
data = this.appendAnalyticsData(data);
data = this.appendSubmissionData(data);
data = this.appendAdditionalInputs(data);
data = this.pruneObject(data);
2、问题出在哪:
  有些重复和啰嗦
  eslint-plugin-smells
  no-reassign
3、重构后代码:
  嵌套的函数调用
  forEach
  reduce
  flow
// 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、糟糕代码:
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);
2、问题出在哪:
  依赖被紧紧的耦合了
  相互调用,耦合!如 product 和 shoppingCart 关系
3、重构后代码:
  dependency injection 依赖注入
  消息经纪人broker
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);
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、糟糕代码:
var search = document.querySelector('.Autocomplete');
search.addEventListener('input', function(e) {
  // Make Ajax call for autocomplete
  console.log(e.target.value);
});
2、问题出在哪:
  会造成卡顿,多余的计算等
3、重构后代码:
  throttle 和 debounce
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、糟糕代码:
var search = document.querySelector('.Autocomplete');
search.addEventListener('input', _.debounce(function(e) {
  // Make Ajax call for autocomplete
  console.log(e.target.value);
}, 500));
2、问题出在哪:
  匿名函数是个好东西,但是给函数命名可以帮助我们:
  Stack Trace(结合Devtools,跟容易debug)
  Dereferencing
  Code Reuse
3、重构后代码:
var search = document.querySelector('.Autocomplete');
search.addEventListener('input', _.debounce(function matches(e) {
  console.log(e.target.value);
}, 500));

十三、未明确的执行
1、糟糕代码:
  明确触发时机而不是,写在 domready
$(document).ready(function() {
  // wire up event handlers

  // declare all the things

  // etc...
});
2、问题出在哪:
  很难做单元测试
3、重构后代码:
  利用单例模块,加上构建器函数
  单例模式(单例有个init 方法,来Kick off) & 构造函数(new Application() -> 在原来的构造函数中Kick off your code!)
(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();
});

十四、双向数据绑定
1、糟糕代码:
  随便看看你们手头的MVVM项目(如Angular的等)
2、问题出在哪:
  很难定位执行顺序和数据流(Hard to track execution & data flow )
  (也是Angular1.x被大力吐槽的地方,被React的Flux)
3、重构后代码:
  方案: flux(action, dispatcher, store->view)
  React Flux
  An Angular2 Todo App: First look at App Development in Angular2

十五、结语
  更多的lint规则,在npm上搜索 eslint-plugin 查找
  Reference

原文链接

著作权归作者所有,商业转载请联系作者获得授权,非商业转载请注明出处。

如社区发表内容存在侵权行为,您可以点击这里查看侵权投诉指引

游戏学院公众号二维码
腾讯游戏学院
微信公众号

提供更专业的游戏知识学习平台