Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

把嵌套的 if else 撸撸平 #28

Open
YIXUNFE opened this issue Nov 1, 2015 · 2 comments
Open

把嵌套的 if else 撸撸平 #28

YIXUNFE opened this issue Nov 1, 2015 · 2 comments

Comments

@YIXUNFE
Copy link
Owner

YIXUNFE commented Nov 1, 2015

把嵌套的 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
Copy link

sKabYY commented Feb 16, 2016

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

@YIXUNFE
Copy link
Owner Author

YIXUNFE commented Feb 19, 2016

@sKabYY 说得对

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants