code_money_guji's Blog

Happy coding

InfoQ:代码之丑 归纳

参考资料:

         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;的方式更为可读?