YIXUNFE / blog

文章区
151 stars 25 forks source link

把嵌套的 if else 撸撸平 #28

Open YIXUNFE opened 8 years ago

YIXUNFE commented 8 years ago

把嵌套的 if else 撸撸平

_我想这篇文章如果有副标题,它应该是:code review 带来的代码优化。_

怎么从一堆的回调中抽身,相信大家已经了解过 promise 等方法。今天我们来看看,我是怎么将代码从一堆 if else 嵌套中解放出来的。


遇到的问题

在我编写一个游戏的时候,键盘(不光是键盘啦)的上下左右键在不同的面板下具有不同的功效,大致如下:

按键 地图界面 命令面板 物品面板 数量面板
向上移动 向上选取一个命令 向上选取一个物品 无作用
向右移动 向右选取一个命令 向右选取一个物品 数量增加
向下移动 向下选取一个命令 向下选取一个物品 无作用
向左移动 向左选取一个命令 向左选取一个物品 数量减少

现在的问题就是,如何能够优雅的实现这些功能?


嵌套的写法

一开始我就直接写了一个使用 if else 嵌套的方法,这里我使用了一个字符串变量 currentStage 表示当前的面板。代码看上去是这样的:

if (keycode === 'up') {
  if (currentStage === 'map') {
    mapPanel.up()
  } else if (currentStage === 'commands') {
    commandsPanel.up()
  } else if (currentStage === 'goods') {
    goodsPanel.up()
  } else if (currentStage === 'number') {
    numberPanel.up()
  }
} else if (keycode === 'right') {
  ...
} else if (keycode === 'down') {
  ...
} else if (keycode === 'left') {
  ...
}

上面的伪代码中,我们先使用了一层 if else 判断了用户的按键,接着又使用了一层 if else 判断在当前面板,以执行各个面板对象的相应方法。看着已经完成了这个功能,但是慢慢的我发现,这种写法将我拖入了一个可怕的维护地狱。因为我每增加一个面板,都需要添加到各个判断按钮的 if else 层中,而在我的游戏中,起码有十几种不同类型的面板,它们都需要按键的支持!这真是太糟糕了。


重构

上面的代码需要重新设计一下了。为了能够让代码更加容易维护,我觉得应该从设置 currentStage 值的方法处着手。

//old function
function setStage (panelObject) {
  currentStage = panelObject.name
}

//new function
function setStage (panelObject) {
  currentStage = panelObject //将 currentStage 直接设置成面板对象
}

我将 currentStage 变量从原来的字符串改变为对象。这下我们的代码简洁多了:

if (keycode === 'up') {
  currentStage.up()
} else if (keycode === 'right') {
  currentStage.right()
} else if (keycode === 'down') {
  currentStage.down()
} else if (keycode === 'left') {
  currentStage.left()
}

利用 currentStage 传递面板对象,直接执行面板对象的事件方法就可以了。但是代码中还有一层判断按键的 if else,我们可以使用 switch 代替。

switch (keycode) {
  case 'up'
     currentStage.up(); break
  case 'right'
     currentStage.right(); break
  case 'down'
     currentStage.down(); break
  case 'left'
     currentStage.left()
}

代码看上去简洁了很多呢,但是不着急,我们还可以继续优化。

我们看到上面的代码中,面板对象都有固定的 upright 等方法,方法名正好和我们的 keycode 值是一致的。

所以我又进行了一次简化:

currentStage[keycode]()

神奇吧,从一堆的 if else 代码到最后只剩下了一行代码。我只能说 code review 是相当重要的。另外我在这里还用上了变量命名的技巧。将面板对象的方法名和 keycode 变量可能的取值命名一致,才能最终得到上面的一行代码。

看到这里可能有人会好奇,为什么我的 keycode 是字符串,而不是事件对象中得到的数字呢?这是因为我的游戏里面还有一个方向摇杆在界面上。用户通过界面操作摇杆(移动端上很方便)或者直接使用键盘,得到的最终结果会被赋值给 keycode,我在这个过程中将 keycode 统一转成了我需要的字符串。


Thanks


sKabYY commented 8 years ago

switch (keycode) { case 'up' currentStage.up() case 'right' currentStage.right() case 'down' currentStage.down() case 'left' currentStage.left() } 这一段要加break

YIXUNFE commented 8 years ago

@sKabYY 说得对