InfoQ:代码之丑 归纳
code_money_guji
posted @ 2011年3月06日 12:53
in 代码重构
, 1595 阅读
参考资料:
http://www.infoq.com/cn/news/2010/11/ugly-code-0 【或者直接google:(InfoQ:代码之丑)】
此文章是关于笔者读infoQ文章所做出的归纳,详细的信息见参考链接.感谢原作者的分享! 由于篇幅, 只是简单的小结.还需要作者原文作为指导.
文中的小结有些事在原文中继续讨论,若有不当,请指出! 谢谢.
Ugly-Code:
if(db.Next()) { return true; } else { return false; } 优化: return db.Next(); ----------------------------------- if(!strcmp(pRec->GetType(), RECTYPE::INSTALL)) { CommDM.ChangGroupInfo(const_cast(CommDM.GetAttr("IPAddress", &(pGroup->m_Attr))), true); } else { CommDM.ChangGroupInfo(const_cast(CommDM.GetAttr("IPAddress", &(pGroup->m_Attr))), false); } 优化为: const char* msg = (0 == retCode ? "Process Success" : "Process Failure"); SendMsg("000", msg, outResult); ----------------------------------- if ( strcmp(rec.type, "PreDropGroupSubs") == 0 || strcmp(rec.type, "StopUserGroupSubsCancel") == 0 || strcmp(rec.type, "QFStopUserGroupSubs") == 0 || strcmp(rec.type, "QFStopUserGroupSubsCancel") == 0 || strcmp(rec.type, "QZStopUserGroupSubs") == 0 || strcmp(rec.type, "QZStopUserGroupSubsCancel") == 0 || strcmp(rec.type, "SQStopUserGroupSubs") == 0 || strcmp(rec.type, "SQStopUserGroupSubsCancel") == 0 || strcmp(rec.type, "StopUseGroupSubs") == 0 || strcmp(rec.type, "PreDropGroupSubsCancel") == 0) 优化: bool shouldExecute(Record& rec) { static const char* execute_type[] = { "PreDropGroupSubs", "StopUserGroupSubsCancel", "QFStopUserGroupSubs", "QFStopUserGroupSubsCancel", "QZStopUserGroupSubs", "QZStopUserGroupSubsCancel", "SQStopUserGroupSubs", "SQStopUserGroupSubsCancel", "StopUseGroupSubs", "PreDropGroupSubsCancel" }; int size = ARRAY_SIZE(execute_type); for (int i = 0; i < size; i++) { if (strcmp(rec.type, execute_type[i]) == 0) { return true; } } return false; } 其实,上面的代码是可以进一步重构的,笔者的理由如下: 当这个常量集合中的其中一个需要修改或者提出,那么当我们进一步的去找这个常量集合的话, 可能会出现代码重构中提及到的"发散式的修改".那么我们有必要将它提出来进行重构. java中,不需要循环,需要的是contains. --------------------------------------- switch(firstChar) { case ‘N’: nextFirstChar = ‘O’; break; case ‘O’: nextFirstChar = ‘P’; break; case ‘P’: nextFirstChar = ‘Q’; break; ...... ...... } 进一步优化: switch(firstChar) { case ‘N’: case ‘O’: case ‘P’: nextFirstChar = firstChar + 1; break; case ‘T’: throw new IllegalArgument(); default: ...... ..... } 进一步优化: if (firstChar >= ‘N’ && firstChar <= ‘S”) { nextFirstChar = firstChar + 1; } else { throw new IllegalArgument(); } 笔者认为,如果上述的switch是无法预测其关系的,那么, 基于key--value的方式..... 并且要注意switch的陷阱,哪些不在switch范围的数值应该如何处理? ------------------------------------------- 还记得Struts1中, 将ActionForm转化为一个Entity实体的做法么? 是直接一大串的setProperty()操作呢,还是使用BeanUtils.copyProperty(..)? 那么, 当这些属性却是是需要在old version中进行操作后copy到一个新的对象中, 笔者认为在重构上, 把copy的代码提出来绝对是好的.但是,是提出到方法 中么 ? 如果提出到当前的类中, 试想,当其他类也完成这种操作的时候,是不是出现了"不同类代码重复"呢? 工具类! 那么,是否应该继承些什么,有一个 默认的操作呢? ------------------------------------------ 不要忘记对参数进行检查, 代码的可行性必须依赖于参数的正确性,否则,再强的代码也会是失败的. 检查代码的时候, throw new YourException. ------------------------------------------ if(attr->status() == STATUS_NEW || attr->status() == STATUS_MODIFIED) { if(strcmp(attr->attrID(), "CallFunc") == 0) { if(0 == strcmp(attr->attrValue(), "1")) { IsCallFunc = 1; } else if(0 == strcmp(attr->attrValue(), "0")) { IsCallFunc = 0; } } 多层次的if语句,是不是应该考虑使用: boolean result = condition1 && condition2;的方式更为可读?