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

绕来绕去，很烧脑

问题现场：

如果单词以辅音开头（或辅音集），把它剩余的步伐移到前面，并且添加上『ay』如pig -> igpay，如果单词以元音开头，保持顺序但是在结尾加上『way』如，egg->eggway等

糟糕代码：

/* 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的配置项：如最大语句数，复杂度，最大嵌套数，最大长度，最多传参，最多嵌套回调

/*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).

测试先行：

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');
    });
  });
});

重构后代码：

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.includes(word[0]);
let startsWithConsonant = word => CONSONANTS.includes(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

粘贴复制

问题现场：

我们需要实现如下的效果

糟糕的代码：

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

从你的JS，TypeScript，C#，Ruby，CSS，HTML等源代码中找到粘贴复制的部分 - JSCPD

重构后代码：

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

复杂的分支语句

糟糕的代码：

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掉。

(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这样的数字

重构后代码：

通过对象

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

代码深渊

糟糕代码：

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

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');
};

脆裂的字符拼接

糟糕代码：

var build = function(id, href, text) {
  return $( "<div id='tab'><a href='" + href + "' id='" + id + "'>" +
    text + "</a></div>" );
}

问题出在哪：

代码很丑陋，也很啰嗦，不直观。

使用 ES6的模板字符串（字符串插值和多行）
很多工具和框架也都提供了响应的支持，如lodash/underscore，angular，react 等

重构后代码：

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>`;

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

绕来绕去，很烧脑

粘贴复制

复杂的分支语句

魔法数字/字符串的坏味道

代码深渊

脆裂的字符拼接

相关推荐

取消回复欢迎你发表评论:

Google 黑客常用搜索语句一览原力计划

npx简介（npxvip是哪国的）

在 Android 模拟器上运行 ARM 应用（android模拟器原理）

GB28181,B接口协议之SIPRTSPRTPRTMP协议从入门到精通

手机实时提取SIM卡打电话的信令和声音-辅助外设与商用通话方案

安装使用Hoppscotch构建API请求访问与测试

轻松转换!AppleNumbers到Excel的快捷教程

Python自动化办公——后台截图（python 自动截图）

电脑端腾讯文档如何导出excel

网络流媒体经典开源软件宝典webRTC, FFMpeg, SIP_流媒体开发教程

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

绕来绕去，很烧脑

粘贴复制

复杂的分支语句

魔法数字/字符串的坏味道

代码深渊

脆裂的字符拼接

相关推荐

取消回复欢迎 你 发表评论:

Google 黑客常用搜索语句一览 原力计划

npx简介（npxvip是哪国的）

在 Android 模拟器上运行 ARM 应用（android模拟器原理）

GB28181,B接口协议之SIPRTSPRTPRTMP协议从入门到精通

手机实时提取SIM卡打电话的信令和声音-辅助外设与商用通话方案

安装使用Hoppscotch构建API请求访问与测试

轻松转换!AppleNumbers到Excel的快捷教程

Python自动化办公——后台截图（python 自动截图）

电脑端腾讯文档如何导出excel

网络流媒体经典开源软件宝典webRTC, FFMpeg, SIP_流媒体开发教程

取消回复欢迎你发表评论:

Google 黑客常用搜索语句一览原力计划