記錄一次重構
記錄一次簡單的重構是為了體現出代碼重構的重要性和緊迫性。如果代碼不能持續進化,那么隨著新的代碼不斷增加,代碼越來越難以維護和擴展,于是老代碼成了難以追蹤、難以理解、一動就崩潰的 bad smell 代碼。此外,不通過持續打磨代碼,程序員自身水平以及團隊水平也難以得到提高。通過重構與 review 機制能夠讓編程經驗與知識在團隊中得以傳遞。
更詳細的關于為什么要重構代碼以及怎樣重構代碼,推薦閱讀經典大作 《重構-改善既有代碼的設計》
現狀
功能
產測工具中有一個檢測觸摸屏的測試,該測試顯示一些可以點擊方塊,點擊方塊或在方塊上滑過就將該方塊變黑,表示該方塊區域觸摸測試通過。
接下來要重構的這一段代碼的功能就是顯示這些可以點擊的方塊,其實現是每一個方塊都是一個獨立的 View 。
代碼
原來的關鍵代碼:
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++);
}
}
效果
這一段代碼存在問題,不能夠適配各種不同的分辨率。它在1072x1448分辨率上僅僅是僥幸能夠工作,而在600x800分辨率的情況下問題就徹底暴露來了。
600x800分辨率下的效果:
評析
代碼格式
該有空格的地方沒有空格,該有分行的地方沒有分行。比如 f (j == 4) continue; 這樣代碼邏輯應該前后加空行以突出。
命名或定義不規范。
- 成員變量 bck 改名為 mDrawables 更為合適;
private Drawable[] bck;
- 成員變量 WIDTH 和 HEIGHT 改名為 mScreenWidth 和 mScreenHeight 更為合適;
- 成員變量 WIDTH_NUM 和 HEIGHT_NUM 應定義為 final static 類型;
private int WIDTH_NUM = 7;
private int HEIGHT_NUM = 9;
- 局部變量 accu 應該是 accumulate 的縮寫,但這里并非求和,修改為 index 更為合適;
- 局部變量 widthLength 、 heightLength 、 crossLength 是寬和高,不應該使用 Length 后綴;
- 局部變量 layoutParams2 ~ 6 和 view2 ~ 6 ,命名通常非常忌諱使用數字后綴作為區分,因為數字后綴不能夠明白地反應出該變量所代表的含義,此外有這樣命名的地方通常存在著重復代碼;
重復代碼
上面提到使用數字后綴區分變量名的地方很可能存在重復代碼,這里確實就存在這樣的問題。局部變量 layoutParams2 ~ 6 其實都是用于同一個目的:作為 addView 的參數將一個方塊( block ) View 添加到 Frame 控件上面去。因此這個添加方塊的過程可以封裝成一個函數。
代碼邏輯
-
錯誤
crossLength因為屏幕的寬和高可能不能夠被塊的行數或列數整除,因此渲染出來的方塊可能會出現不連續的情形(見前面600x800分辨率下的效果圖)。代碼中試圖用 crossLength 來調整這樣的情況,但邏輯上確實錯誤的;
-
復雜
代碼使用兩個單獨 for 循環單獨根據行和列來添加方塊,并且在其中判斷魔數 j == 4 來處理,理解起來非常費勁;
-
重構
如果有豐富的編碼或重構經驗,我們可以根據上面評析出來的問題一步到位把代碼重構好。但通常不建議這么做,心急吃不了熱豆腐。重構通常是:
- 小步前進
- 每前進一步,都要回歸測試
- 先易后難,往往從代碼格式、規范命名、從重復代碼中提煉功能函數入手
下面就根據上面的重構原則暫進地重構上面的代碼
規范命名
根據前面列出的命令或定義不規范問題的修改意見重構 version1.0 如下:
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++);
}
}
重構命名之后,代碼閱讀起來稍稍容易明白一點。但效果不大,接下來就要靠減少重復代碼,提煉功能函數了。
提煉功能函數
提煉功能函數之后的 version2.0 閱讀效果就大為改觀了:
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);
}
}
但上面的代碼在邏輯還是存在錯誤,要修改邏輯錯誤,就必現修改添加方塊的算法。
改善算法
根據功能需求,需要9x9大小的方格表的四周和對角線上提交可點擊的方塊。因此判斷一個表格位置是否需要添加方塊就是判斷該表格是不是處于四周或對角線上,這個判斷很容易就能做到。于是有了下面的 version3.0 :
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);
}
}
}
}
持續重構
到目前為止 version3.0 已經能夠正常工作,并且代碼邏輯也比較清晰易于閱讀。但如果從更宏觀的角度來看,還存在重構的空間,比如:現在每一個方塊都是一個獨立的 View ,其實可以使用自定義 View ,通過改寫 onDraw 來自己描繪每一個方塊。
來自:http://blog.csdn.net/kesalin/article/details/77851167