记录一次重构
tw3403
7年前
<p>记录一次简单的重构是为了体现出代码重构的重要性和紧迫性。如果代码不能持续进化，那么随着新的代码不断增加，代码越来越难以维护和扩展，于是老代码成了难以追踪、难以理解、一动就崩溃的 bad smell 代码。此外，不通过持续打磨代码，程序员自身水平以及团队水平也难以得到提高。通过重构与 review 机制能够让编程经验与知识在团队中得以传递。</p> <p>更详细的关于为什么要重构代码以及怎样重构代码，推荐阅读经典大作 <a href="/misc/goto?guid=4959754361600802445" rel="nofollow,noindex">《重构-改善既有代码的设计》</a></p> <h2>现状</h2> <h3>功能</h3> <p>产测工具中有一个检测触摸屏的测试，该测试显示一些可以点击方块，点击方块或在方块上滑过就将该方块变黑，表示该方块区域触摸测试通过。</p> <p>接下来要重构的这一段代码的功能就是显示这些可以点击的方块，其实现是每一个方块都是一个独立的 View 。</p> <h3>代码</h3> <p>原来的关键代码：</p> <pre> <code class="language-java">private void showBlock(){ int widthLength = WIDTH / WIDTH_NUM; int heightLength = HEIGHT / HEIGHT_NUM; int crossLength = (WIDTH - 2 * widthLength)/(HEIGHT_NUM - 2); int accu = 0; for(int i = 0; i < WIDTH_NUM; i++) { FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams(widthLength, heightLength); layoutParams.leftMargin = i * widthLength; layoutParams.topMargin = 0; TouchView view = new TouchView(mContext); mFrame.addView(view, layoutParams); view.init(accu, TouchView.DISTOGGLED, bck, layoutParams, listener); mTouchData.add(accu++); FrameLayout.LayoutParams layoutParams2 = new FrameLayout.LayoutParams(widthLength, heightLength); layoutParams2.leftMargin = i * widthLength; layoutParams2.topMargin = (HEIGHT_NUM - 1) * heightLength; TouchView view2 = new TouchView(mContext); mFrame.addView(view2, layoutParams2); view2.init(accu, TouchView.DISTOGGLED, bck, layoutParams2, listener); mTouchData.add(accu++); } for(int j = 1; j < HEIGHT_NUM - 1; j++) { FrameLayout.LayoutParams layoutParams3 = new FrameLayout.LayoutParams(widthLength, heightLength); layoutParams3.leftMargin = 0; layoutParams3.topMargin = j * heightLength; TouchView view3 = new TouchView(mContext); mFrame.addView(view3, layoutParams3); view3.init(accu, TouchView.DISTOGGLED, bck, layoutParams3, listener); mTouchData.add(accu++); FrameLayout.LayoutParams layoutParams4 = new FrameLayout.LayoutParams(widthLength, heightLength); layoutParams4.leftMargin = (WIDTH_NUM - 1) * widthLength; layoutParams4.topMargin = j * heightLength; TouchView view4 = new TouchView(mContext); mFrame.addView(view4, layoutParams4); view4.init(accu, TouchView.DISTOGGLED, bck, layoutParams4, listener); mTouchData.add(accu++); FrameLayout.LayoutParams layoutParams5 = new FrameLayout.LayoutParams(crossLength, heightLength); layoutParams5.leftMargin = widthLength + (j - 1) * crossLength; layoutParams5.topMargin = j * heightLength; TouchView view5 = new TouchView(mContext); mFrame.addView(view5, layoutParams5); view5.init(accu, TouchView.DISTOGGLED, bck, layoutParams5, listener); mTouchData.add(accu++); if(j == 4) continue; FrameLayout.LayoutParams layoutParams6 = new FrameLayout.LayoutParams(crossLength, heightLength); layoutParams6.leftMargin = WIDTH - (widthLength + j * crossLength); layoutParams6.topMargin = j * heightLength; TouchView view6 = new TouchView(mContext); mFrame.addView(view6, layoutParams6); view6.init(accu, TouchView.DISTOGGLED, bck, layoutParams6, listener); mTouchData.add(accu++); } }</code></pre> <h3>效果</h3> <p>这一段代码存在问题，不能够适配各种不同的分辨率。它在1072x1448分辨率上仅仅是侥幸能够工作，而在600x800分辨率的情况下问题就彻底暴露来了。</p> <p>600x800分辨率下的效果：</p> <p><img src="https://simg.open-open.com/show/5f3d6be346068ed2b6f3665b20da1388.png"></p> <h2>评析</h2> <h3>代码格式</h3> <p>该有空格的地方没有空格，该有分行的地方没有分行。比如 f (j == 4) continue; 这样代码逻辑应该前后加空行以突出。</p> <h3>命名或定义不规范。</h3> <ul> <li>成员变量 bck 改名为 mDrawables 更为合适；</li> </ul> <pre> <code class="language-java">private Drawable[] bck;</code></pre> <ul> <li>成员变量 WIDTH 和 HEIGHT 改名为 mScreenWidth 和 mScreenHeight 更为合适；</li> <li>成员变量 WIDTH_NUM 和 HEIGHT_NUM 应定义为 final static 类型；</li> </ul> <pre> <code class="language-java">private int WIDTH_NUM = 7; private int HEIGHT_NUM = 9;</code></pre> <ul> <li>局部变量 accu 应该是 accumulate 的缩写，但这里并非求和，修改为 index 更为合适；</li> <li>局部变量 widthLength 、 heightLength 、 crossLength 是宽和高，不应该使用 Length 后缀；</li> <li>局部变量 layoutParams2 ~ 6 和 view2 ~ 6 ，命名通常非常忌讳使用数字后缀作为区分，因为数字后缀不能够明白地反应出该变量所代表的含义，此外有这样命名的地方通常存在着重复代码；</li> </ul> <h3>重复代码</h3> <p>上面提到使用数字后缀区分变量名的地方很可能存在重复代码，这里确实就存在这样的问题。局部变量 layoutParams2 ~ 6 其实都是用于同一个目的：作为 addView 的参数将一个方块( block ) View 添加到 Frame 控件上面去。因此这个添加方块的过程可以封装成一个函数。</p> <h3>代码逻辑</h3> <ul> <li> <p>错误</p> crossLength <p>因为屏幕的宽和高可能不能够被块的行数或列数整除，因此渲染出来的方块可能会出现不连续的情形（见前面600x800分辨率下的效果图）。代码中试图用 crossLength 来调整这样的情况，但逻辑上确实错误的；</p> <ul> <li> <p>复杂</p> <p>代码使用两个单独 for 循环单独根据行和列来添加方块，并且在其中判断魔数 j == 4 来处理，理解起来非常费劲；</p> </li> </ul> </li> </ul> <h2>重构</h2> <p>如果有丰富的编码或重构经验，我们可以根据上面评析出来的问题一步到位把代码重构好。但通常不建议这么做，心急吃不了热豆腐。重构通常是：</p> <ul> <li>小步前进</li> <li>每前进一步，都要回归测试</li> <li>先易后难，往往从代码格式、规范命名、从重复代码中提炼功能函数入手</li> </ul> <p>下面就根据上面的重构原则暂进地重构上面的代码</p> <h3>规范命名</h3> <p>根据前面列出的命令或定义不规范问题的修改意见重构 version1.0 如下：</p> <pre> <code class="language-java">private void showBlock() { int width = mScreenWidth / WIDTH_NUM; int height = mScreenHeight / HEIGHT_NUM; int crossWidth = (mScreenWidth - 2 * width)/(HEIGHT_NUM - 2); int index = 0; for (int i = 0; i < WIDTH_NUM; i++) { FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams(width, height); layoutParams.leftMargin = i * width; layoutParams.topMargin = 0; TouchView view = new TouchView(mContext); mFrame.addView(view, layoutParams); view.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams, listener); mTouchData.add(index++); FrameLayout.LayoutParams layoutParams2 = new FrameLayout.LayoutParams(width, height); layoutParams2.leftMargin = i * width; layoutParams2.topMargin = (HEIGHT_NUM - 1) * height; TouchView view2 = new TouchView(mContext); mFrame.addView(view2, layoutParams2); view2.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams2, listener); mTouchData.add(index++); } for (int j = 1; j < HEIGHT_NUM - 1; j++) { FrameLayout.LayoutParams layoutParams3 = new FrameLayout.LayoutParams(width, height); layoutParams3.leftMargin = 0; layoutParams3.topMargin = j * height; TouchView view3 = new TouchView(mContext); mFrame.addView(view3, layoutParams3); view3.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams3, listener); mTouchData.add(index++); FrameLayout.LayoutParams layoutParams4 = new FrameLayout.LayoutParams(width, height); layoutParams4.leftMargin = (WIDTH_NUM - 1) * width; layoutParams4.topMargin = j * height; TouchView view4 = new TouchView(mContext); mFrame.addView(view4, layoutParams4); view4.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams4, listener); mTouchData.add(index++); FrameLayout.LayoutParams layoutParams5 = new FrameLayout.LayoutParams(crossWidth, height); layoutParams5.leftMargin = width + (j - 1) * crossWidth; layoutParams5.topMargin = j * height; TouchView view5 = new TouchView(mContext); mFrame.addView(view5, layoutParams5); view5.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams5, listener); mTouchData.add(index++); if (j == 4) continue; FrameLayout.LayoutParams layoutParams6 = new FrameLayout.LayoutParams(crossWidth, height); layoutParams6.leftMargin = mScreenWidth - (width + j * crossWidth); layoutParams6.topMargin = j * height; TouchView view6 = new TouchView(mContext); mFrame.addView(view6, layoutParams6); view6.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams6, listener); mTouchData.add(index++); } }</code></pre> <p>重构命名之后，代码阅读起来稍稍容易明白一点。但效果不大，接下来就要靠减少重复代码，提炼功能函数了。</p> <h3>提炼功能函数</h3> <p>提炼功能函数之后的 version2.0 阅读效果就大为改观了：</p> <pre> <code class="language-java">private void addBlock(int index, int x, int y, int w, int h) { FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams(w, h); layoutParams.leftMargin = x; layoutParams.topMargin = y; TouchView view = new TouchView(mContext); mFrame.addView(view, layoutParams); view.init(index, TouchView.DISTOGGLED, mDrawables, layoutParams, listener); mTouchData.add(index); } private void showBlock() { int width = mScreenWidth / WIDTH_NUM; int height = mScreenHeight / HEIGHT_NUM; int crossWidth = (mScreenWidth - 2 * width)/(HEIGHT_NUM - 2); int index = 0; int x, y; for (int i = 0; i < WIDTH_NUM; i++) { x = i * width; y = 0; addBlock(index++, x, y, width, height); x = i * width; y = (HEIGHT_NUM - 1) * height; addBlock(index++, x, y, width, height); } for (int j = 1; j < HEIGHT_NUM - 1; j++) { x = (WIDTH_NUM - 1) * width; y = j * height; addBlock(index++, x, y, width, height); x = width + (j - 1) * crossWidth; y = j * height; addBlock(index++, x, y, crossWidth, height); if (j == 4) continue; x = mScreenWidth - (width + j * crossWidth); y = j * height; addBlock(index++, x, y, crossWidth, height); } }</code></pre> <p>但上面的代码在逻辑还是存在错误，要修改逻辑错误，就必现修改添加方块的算法。</p> <h3>改善算法</h3> <p>根据功能需求，需要9x9大小的方格表的四周和对角线上提交可点击的方块。因此判断一个表格位置是否需要添加方块就是判断该表格是不是处于四周或对角线上，这个判断很容易就能做到。于是有了下面的 version3.0 ：</p> <pre> <code class="language-java">private final static int MAX_NUM = 9; private void addBlock(int index, int x, int y, int w, int h) { FrameLayout.LayoutParams lp = new FrameLayout.LayoutParams(w, h); lp.leftMargin = x; lp.topMargin = y; TouchView view = new TouchView(mContext); view.init(index, TouchView.DISTOGGLED, mDrawables, lp, mToggledListener); mFrame.addView(view, lp); } private void showBlock() { int maxIndex = MAX_NUM - 1; int itemWidth = mScreenWidth / MAX_NUM; int itemHeight = mScreenHeight / MAX_NUM; int index = 0; for (int i = 0; i <= maxIndex; ++i) { for (int j = 0; j <= maxIndex; ++j) { boolean show = (i == 0 || i == maxIndex || j == 0 || j == maxIndex) || (i == j) || (i == (maxIndex - j)); if (show) { int x = i * itemWidth; int y = j * itemHeight; int w = itemWidth; int h = itemHeight; if (i == maxIndex) { w = mScreenWidth - maxIndex * itemWidth; } if (j == maxIndex) { h = mScreenHeight - maxIndex * itemHeight; } addBlock(index++, x, y, w, h); } } } }</code></pre> <h2>持续重构</h2> <p>到目前为止 version3.0 已经能够正常工作，并且代码逻辑也比较清晰易于阅读。但如果从更宏观的角度来看，还存在重构的空间，比如：现在每一个方块都是一个独立的 View ，其实可以使用自定义 View ，通过改写 onDraw 来自己描绘每一个方块。</p> <p> </p> <p>来自：http://blog.csdn.net/kesalin/article/details/77851167</p> <p> </p>