重构实践——重构的小例子

于 2012-05-27 14:58:00 发布 · 279 阅读 · 1

最近一直在看关于重构的经典：《重构——改善既有代码的设计》，也算是颇有所得。一直想写写感悟，奈何书写的实在太好，感悟的都是里面的原话，实在没必要。昨天同学写了一个简单的小程序，让我帮忙找bug。我一看，觉得这段代码非常适合进行练手，就要过来，花了点时间重构一下。下面进入正题。

程序的目的是，计算两个十六进制数的结果。

输入：

char a[10]= {'0','0','0','0','0','0','0','0','9','1'};

char b[10]= {'0','0','0','0','0','0','0','0','8','7'};

输出：

result[10]:{'0','0','0','0','0','0','0','1','1','8'};

当计算有溢出时提示。

原代码如下：

#include <iostream>

using namespace std;

int hexCharToInt(char c)
{
    //十六进制字符转十进制整数,a~f大小写均可
    if (c >= '0' && c <= '9') return (c - '0');
    if (c >= 'A' && c <= 'F') return (c - 'A' + 10);
    if (c >= 'a' && c <= 'f') return (c - 'a' + 10);
    return 0;
}

char intToHexChar(int a)
{
    //0-15的十进制整数转十六进制字符
    if (a > 9)
    {
        return static_cast<char>('A' + a - 10);
    }
    return static_cast<char>(a + '1' - 1);
}
char* addHex(char* a,char* b)
{
    //两个10位的用数组存储的十六进制数相加
    char* result=new char[10];
    int intA=0;
    int intB=0;
    int sum=0;
    bool iscarry=false;
    for(int i=9; i>=0; i--)
    {
        intA=hexCharToInt(a[i]);
        intB=hexCharToInt(b[i]);
        sum=intA+intB;

        if(sum>=16)
        {
            if(iscarry==false)
            {
                result[i]=intToHexChar(sum-16);
            }
            else
            {
                result[i]=intToHexChar(sum-15);
            }

            iscarry=true;

        }
        else
        {
            if(iscarry==false)
            {
                result[i]=intToHexChar(sum);
            }
            else
            {
                result[i]=intToHexChar(sum+1);
            }
            iscarry=false;
        }
    }
    if(iscarry==true)
    {
        cout<<"addition overflow"<<endl;
    }
    return result;
}

int sum(char a,char b){
    intA=hexCharToInt(a);
    intB=hexCharToInt(b);
    return intA+intB;
}

int main()
{
    char a[10]= {'F','0','0','0','0','0','0','0','9','1'};
    char b[10]= {'1','0','0','0','0','0','0','0','8','7'};
    char* r=addHex(a,b);
    cout<<r;
    return 0;
}






这段代码的“坏味道”是，在addHex函数里，存在Long Method和Data Clumps的问题。而且，很明显条件语句逻辑较为复杂。



首先检查比较简单的东西。在addHex的最后一段，




 if(iscarry==true)
    {
        cout<<"addition overflow"<<endl;
    }
    return result;
可以通过Extract Method方法提出来作为单独函数：





void checkOverFlow(bool iscarry){
    if(iscarry)
    {
        cout<<"addition overflow"<<endl;
    }
}
于是addHex函数就变成了：
char* addHex(char* a,char* b)
{
    //两个10位的用数组存储的十六进制数相加
    char* result=new char[10];
    int intA=0;
    int intB=0;
    int sum=0;
    bool iscarry=false;
    for(int i=9; i>=0; i--)
    {
        intA=hexCharToInt(a[i]);
        intB=hexCharToInt(b[i]);
        sum=intA+intB;

        if(sum>=16)
        {
            if(iscarry==false)
            {
                result[i]=intToHexChar(sum-16);
            }
            else
            {
                result[i]=intToHexChar(sum-15);
            }

            iscarry=true;

        }
        else
        {
            if(iscarry==false)
            {
                result[i]=intToHexChar(sum);
            }
            else
            {
                result[i]=intToHexChar(sum+1);
            }
            iscarry=false;
        }
    }
   
    checkOverFlow(iscarry);

    return result;
}
下面再看数据泥团。



addHex函数里声明了五个变量：result;intA;intB;sum;iscarry;其中，intA和intB都只在for循环里用了一回，完全没有必要，而且会阻碍对循环体的重构。


可以通过Inline Temp方法消除：找到所有使用到intA和intB的地方，将之修改为对它赋值的表达式：




sum=hexCharToInt(a[i])+hexCharToInt(b[i]);



这样，就可以消除掉intA和intB这两个临时变量了。



另外sum也没有必要在for循环外部声明，将之移到里面来，使for循环体与外面的联系尽可能小：




int sum=hexCharToInt(a[i])+hexCharToInt(b[i]);



这样，addHex函数就变成了：



char* addHex(char* a,char* b)
{
    //两个10位的用数组存储的十六进制数相加
    char* result=new char[10];
    bool iscarry=false;
    for(int i=9; i>=0; i--)
    {
        int sum=hexCharToInt(a[i])+hexCharToInt(b[i]);

        if(sum>=16)
        {
            if(iscarry==false)
            {
                result[i]=intToHexChar(sum-16);
            }
            else
            {
                result[i]=intToHexChar(sum-15);
            }

            iscarry=true;

        }
        else
        {
            if(iscarry==false)
            {
                result[i]=intToHexChar(sum);
            }
            else
            {
                result[i]=intToHexChar(sum+1);
            }
            iscarry=false;
        }
    }
    checkOverFlow(iscarry);
    return result;

}




下面要对for循环里的东西进行操作了。



for循环主要做了两件事：给result[i]赋值，和检查是否需要进位。将这两件事分别提取出函数：


给result赋值：




int getChar(int sum,bool iscarry){
    int result;
    
    if(sum>=16)
    {
        if(iscarry==false)
        {
            resul=intToHexChar(sum-16);
        }
        else
        {
            result=intToHexChar(sum-15);
        }
    }
    else
    {
        if(iscarry==false)
        {
            result=intToHexChar(sum);
        }
        else
        {
            result=intToHexChar(sum+1);
        }
    }
    return result;
}




检查是否进位：





bool isCarry(int sum){
    return (sum>=16)?true:false;
}

addHex函数精简为：





char* addHex(char* a,char* b)
{
    //两个10位的用数组存储的十六进制数相加
    char* result=new char[10];
    bool iscarry=false;
    for(int i=9; i>=0; i--)
    {
	int sum=hexCharToInt(a[i])+hexCharToInt(b[i]);
        result[i] = addChar(sum,iscarry);
	iscarry = isCarry(sum);
    }
   
    checkOverFlow(iscarry);

    return result;
}
看起来是不是舒服了许多？



在getChar()函数里，还有一个问题，就是嵌套太多，比较混乱。里面有两个开关：sum和iscarry。怎么减少复杂度呢？


我们观察一下，其实sum和iscarry是有关的，iscarry的含义是进位，它的用处就是将sum加一。因为有了这个关系，我们就可以通过一个表达式来将这两个开关合并为一个：




sum = sum + (iscarry?1:0);
这样，getChar函数就变成了：





int getChar(int sum,bool iscarry){
    int result;
    sum = sum + (iscarry?1:0);
    if(sum>=16)
    {
        result=intToHexChar(sum-16);
    }
    else
    {
        result=intToHexChar(sum);
    }
    return result;
}
其实if语句还可以精简为：
result = (sum>=16)?intToHexChar(sum-16):intToHexChar(sum);
再来一步：
result = intToHexChar(sum%16);
这样，getChar函数被进一步精简为：





int getChar(int sum,bool iscarry){
    int result;
    sum = sum + (iscarry?1:0);
    result = intToHexChar(sum%16);
    return result;
}
还有一些小的地方，比如在int hexCharToInt()的代码：





if (c >= '0' && c <= '9') return (c - '0');
不符合我们的习惯，一般我们都是用'0'<=c<='9'这样的方式，因此重写为
if ('0' <=c && c <= '9') return (c - '0');
最近，完成的代码是这样的：





#include <iostream>

using namespace std;

int getChar(int sum,bool iscarry);

bool isCarry(int sum){
    return (sum>=16)?true:false;
}

void checkOverFlow(bool iscarry){
    if(iscarry)
    {
        cout<<"addition overflow"<<endl;
    }
}

int hexCharToInt(char c)
{
    //十六进制字符转十进制整数,a~f大小写均可
    if ('0' <= c && c <= '9') return (c - '0');
    if ('A' <= c && c <= 'F') return (c - 'A' + 10);
    if ('a' <= c && c <= 'f') return (c - 'a' + 10);
    return 0;
}

char intToHexChar(int a)
{
    //0-15的十进制整数转十六进制字符
    if (a > 9)
    {
        return static_cast<char>('A' + a - 10);
    }
    return static_cast<char>(a + '1' - 1);
}

char* addHex(char* a,char* b)
{
    //两个10位的用数组存储的十六进制数相加
    char* result=new char[10];
    bool iscarry=false;
    for(int i=9; i>=0; i--)
    {
        int sum=hexCharToInt(a[i])+hexCharToInt(b[i]);
        result[i] = getChar(sum,iscarry);
        iscarry = isCarry(sum);
    }

    checkOverFlow(iscarry);

    return result;
}

int getChar(int sum,bool iscarry){
    int result;
    sum = sum + (iscarry?1:0);
    result = intToHexChar(sum%16);
    return result;
}

int main()
{
    char a[10]= {'0','0','0','0','0','0','0','0','9','1'};
    char b[10]= {'0','0','0','0','0','0','0','0','8','7'};
    char* r=addHex(a,b);
    cout<<r;
    return 0;
}

比刚开始的代码好多了吧？



一些感想：一定要小步前进，每前进一步都要进行测试，通过了再进行下一步。如果一次改动太大的话，万一哪个地方出现错误，想找出来比较麻烦。还有，函数的名字一定要起的好，根据它的用处来取名。我取的名字还不太好，以后要多注意。


另外，对于如何减少复杂嵌套，我还没有搞定。以后多研究研究。


在重构的过程中，出现一点小问题，在




sum = sum + (iscarry?1:0);
这句话中，我刚开始是这样写的：





sum = sum + iscarry?1:0;
结果出现错误。经检查发现是由于三元操作符的优先级很低，导致先进行了加法，再进行三元操作符运算。



代码是跑在Code Block里的。Code Block是今天第一次用的IDE，感觉很不错，比较轻量，比MS的VS轻多了。好像还跨平台。






























                

        




    

    
  



    

      

        

            

              
                
                  iteye_12332
                
              
            

            

                关注
              
            

        

        

          
      

      










            

  



          




        



    





    



      

      

        
      

      





	

		评论	
	

	
	




  

    

      添加红包
      
    

    

      

        
        

          
          
        

        
请填写红包祝福语或标题

      

      

        
        

          
          个
        

        
红包个数最小为10个

      

      

        
        

          
          元
        

        
红包金额最低5元

      

      

        
        

          当前余额3.43元
          前往充值 >
        

      

      

        

          需支付：10.00元
        

        
        
      

    

  





  

    
    
  

  

    
    
  








  

    

      

        

          

            
            
成就一亿技术人!

          

          

        

        

          

          

            领取后你会自动成为博主和红包主的粉丝
            规则
          

        

      

      

        

          

            
              
            
            

              
hope_wisdom
 发出的红包
            

          

        

        

          

          

          

        

      

    

    

  



      
      

      
实付元

      
使用余额支付

      

        

          

            
            点击重新获取
          

        

        
扫码支付

      

      

        
          
            
          
          
        
      

      

        
        钱包余额
          0
          

            
            

              

                
抵扣说明：

                
 1.余额是钱包充值的虚拟货币，按照1:1的比例进行支付金额的抵扣。
 2.余额无法直接购买下载，可以购买VIP、付费专栏及课程。

              

            

          

      

      余额充值