代碼之丑(二)——長長的條件
這是一個長長的判斷條件:
|| strcmp(type, "CancelUserGroup") == 0
|| strcmp(type, "QFUserGroup") == 0
|| strcmp(type, "CancelQFUserGroup") == 0
|| strcmp(type, "QZUserGroup") == 0
|| strcmp(type, "CancelQZUserGroup") == 0
|| strcmp(type, "SQUserGroup") == 0
|| strcmp(type, "CancelSQUserGroup") == 0
|| strcmp(type, “UseGroup") == 0
|| strcmp(type, "CancelGroup") == 0)
之所以注意到它,因為最后兩個條件是在最新修改里面加入的,換句話說,這不是一次寫就的代碼。單就這一次而言,只改了兩行,這是可以接受的。但這是遺留代碼,每次可能只改了一兩行,通常我們會不只一次踏入這片土地。經年累月,代碼成了這個樣子。
就我接觸過的代碼而言,這并不是最長的判斷條件。這種代碼極大的開拓了我的視野,現在的我,即便面前是一屏無法容納的條件,也可以坦然面對了,雖然顯示器越來越大。
其實,如果這個判斷條件是這個函數里僅有的東西,我也是可以接受的。遺憾的是,大多數情況下,這只不過是一個更大函數中的一小段而已。為了讓這段代碼可以接受一些,我們不妨稍做封裝:
return (strcmp(type, “DropGroup") == 0
|| strcmp(type, "CancelUserGroup") == 0
|| strcmp(type, "QFUserGroup") == 0
|| strcmp(type, "CancelQFUserGroup") == 0
|| strcmp(type, "QZUserGroup") == 0
|| strcmp(type, "CancelQZUserGroup") == 0
|| strcmp(type, "SQUserGroup") == 0
|| strcmp(type, "CancelSQUserGroup") == 0
|| strcmp(type, “UseGroup") == 0
|| strcmp(type, "CancelGroup") == 0); }
if (shouldExecute(type)) { ... }
現在,雖然條件依然還是很多,但比起原來龐大的函數,至少它已經被控制在一個相對較小的函數里了。更重要的是,通過函數名,我們終于有機會告訴世人這段代碼判斷的是什么了。
雖然提取函數把這段代碼混亂的條件分離開來,它還是可以繼續改進的。比如,我們把判斷的條件進一步提取:
static const char* execute_type[] = {
"DropGroup", "CancelUserGroup",
"QFUserGroup", "CancelQFUserGroup", "QZUserGroup",
"CancelQZUserGroup", "SQUserGroup",
"CancelSQUserGroup", "UseGroup", "CancelGroup" };
int size = ARRAY_SIZE(execute_type);
for (int i = 0; i < size; i++) {
if (strcmp(type, execute_type[i]) == 0) {
return true; } }
return false;}
這樣的話,如果以后要加一個新的type,只要在數組中增加一個新的元素即可。這段代碼還可以進一步封裝,把這個type列表變成聲明式,進一步提高代碼的可讀性。
簡單的理解聲明式的風格,就是描述做什么,而不是怎么做。一個聲明式編程的例子是Rails里面的數據關聯,為人熟知的has_many和belongs_to。通過這個聲明,模型類就會具備一些數據關聯的能力。具體到實際開發里,聲明式編程需要有兩個部分:一方面是一些基礎的框架性代碼,另一方面是應用層面如何使用。通常,框架代碼不像應用層面代碼那么好理解,但有了這個基礎,應用代碼就會變得簡單許多。針對上面那段代碼,按照這種風格,我改造了代碼,下面是框架部分的代碼:
\bool predicate_name(const char* field) {
\ static const char* predicate_true_fields[] = {
#define STR_PREDICATE_ITEM(item) #item ,#define END_STR_PREDICATE \
};\ \
int size = ARRAY_SIZE(predicate_true_fields);\
for (int i = 0; i < size; i++) { \
if (strcmp(field, predicate_true_fields[i]) == 0) {\
return true;\
}\ }\\
return false;\}
這里用到了C/C++常見的宏技巧,為的就是讓應用層面的代碼寫起來更像聲明。稍微對比一下,就會發現,實際上二者幾乎是一樣的。有了框架,就該應用了:
STR_PREDICATE_ITEM(DropGroup )
STR_PREDICATE_ITEM(CancelUserGroup )
STR_PREDICATE_ITEM(QFUserGroup )
STR_PREDICATE_ITEM(CancelQFUserGroup )
STR_PREDICATE_ITEM(QZUserGroup )
STR_PREDICATE_ITEM(CancelQZUserGroup )
STR_PREDICATE_ITEM(SQUserGroup )
STR_PREDICATE_ITEM(CancelSQUserGroup )
STR_PREDICATE_ITEM(UseGroup )
STR_PREDICATE_ITEM(CancelGroup )END_STR_PREDICATE
shouldExecute就此重現出來了。不過,這段代碼已經不再像一個函數,而更像一段聲明,而這,恰恰就是我們的目標。有了這個基礎,實現一個新的函數,不過是做一個新的聲明而已。
使用這個新函數的方法依然如故:
雖然應用代碼變得簡單了,但寫出框架的結構是需要一定基礎的。它不像應用代碼那樣來得平鋪直敘,但其實也沒那么難,只不過很多人從沒有考慮把代碼寫成這樣。只要換個角度去思考,多多練習,也就可以駕輕就熟了。
發現這種代碼很容易,只要看到在長長的判斷條件,就是它了。要限制這種代碼的存在,只要以設定一個簡單的規則:
- 判斷條件里面不允許多個條件的組合
在實際的應用中,我們會把“3”定義為“多”,也就是如果有兩個條件的組合,可以接受,如果是三個,還是改吧!
雖然通過不斷調整,這段代碼已經不同于之前,但它依然不是我們心目中的理想代碼。出現這種代碼,往往意味背后有更嚴重的設計問題。不過,它并不是這里討論的內容,這里的討論就到此為止吧!
相關文章:代碼之丑——讓判斷條件做真正的選擇
留言列表