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

最新推荐文章于 2022-08-30 21:53:22 发布

iteye_12332

最新推荐文章于 2022-08-30 21:53:22 发布

阅读量279

点赞数

最近一直在看关于重构的经典：《重构——改善既有代码的设计》，也算是颇有所得。一直想写写感悟，奈何书写的实在太好，感悟的都是里面的原话，实在没必要。昨天同学写了一个简单的小程序，让我帮忙找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;
}

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

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

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

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