分数的加减法——C语言初学者代码中的常见错误与瑕疵(11)

前文链接：分数的加减法——C语言初学者代码中的常见错误与瑕疵(10)

原代码2

#include <stdio.h>
#include <string.h>

int abs(int a)
{
return a>0?a:-a;
}

int gcd(int a,int b)
{
if(a<b)
a^=b^=a^=b;
return b?gcd(b,a%b):a;
}

int main()
{
int a,b,c,d,temp,temp0,temp1;
char op;
while(scanf("%d/%d%c%d/%d",&a,&b,&op,&c,&d)!=EOF)
{
temp1=b*d/gcd(b,d);
if(op=='-')
temp0=a*temp1/b-c*temp1/d;
else
temp0=a*temp1/b+c*temp1/d;
temp=gcd(abs(temp0),temp1);
temp0/=temp;
temp1/=temp;
if(temp0&&temp1==1)
printf("%d\n",temp0);
else if(temp0)
printf("%d/%d\n",temp0,temp1);
else
printf("0\n");
}
return 1;
}

评析

　　这是另一位初学者给出的代码。
　　这段代码貌似更简练一些，但也有很多毛病。

总体

　　总体结构上偷工减料，贪小便宜(省写函数类型声明)，且次序不当（把main()放在后面）。

#include <string.h>

　　这是有鱼没鱼先撒一网。实际上多余写这条，因为根本就没用。程序员应该知道什么叫Occam's razor：Plurality should not be posited without necessity.(拉丁文表述是：Pluralitas non est ponenda sine neccesitate.)。

main()

int a,b,c,d,temp,temp0,temp1;

　　a,b,c,d这几个标识符过于庸俗，不过考虑到直接来自于题目，似乎也不便深责。
　　temp,temp0,temp1实在是太差了，无厘头。而且完全不应该定义在这个位置，应该放在while的循环体内定义。

char op;

　　这个没什么问题，op显然是operation的缩写。

while(scanf("%d/%d%c%d/%d",&a,&b,&op,&c,&d)!=EOF)

　　这个scanf()函数调用的转换格式——"%d/%d%c%d/%d"，或者叫“匹配格式”设计得不好。

　　如果要严格满足题目中“a, b, c, d是一个0-9的整数”的要求，这个格式应该写为"%1d/%1d%c%1d/%1d"。更投机取巧的写法可以把那个正负号视为c的一部分："%1d/%1d%2d/%1d"，但这仅仅在加减法时才适用，并不具备一般性。

　　如果希望程序更“宽容”一些，比如能接受输入流中合情合理的空格，例如 1 / 8 + 3 / 8，这个转换格式应该设计为"%d / %d %c %d / %d"。

　　对于"%d/%d%c%d/%d"来说， 1 / 8 + 3 / 8 这样的输入最多只能转换一个"%d"，即正确输入那个1。而对于"%d / %d %c %d / %d"来说， 1 / 8 + 3 / 8这样的输入能完全地正确转换(甚至多加几个空白字符也没关系)。

　　这是因为对于scanf()来说，转换格式中的空白字符(white-space character)的意义是读至第一个非white-space character。(A directive composed of white-space character(s) is executed by reading input up to the first non-white-space character (which remains unread), or until no more characters can be read. The directive nev er fails.)

　　一般情况下，在scanf()的转换控制字符串中应该不写任何white-space character或非white-space character(怎样利用scanf()函数自虐 )，但是凡事都有例外。

temp1=b*d/gcd(b,d);

急躁。急吼吼地就开始处理公共细节。

粗心。显然没考虑b或d为0的情况。

if(op=='-')
temp0=a*temp1/b-c*temp1/d;
else
temp0=a*temp1/b+c*temp1/d;

　　显得有点笨重。要是我也许会这样写

if(op=='-')
c = - c ;
temp0=a*temp1/b+c*temp1/d;

temp=gcd(abs(temp0),temp1);

　　求绝对值这种小事情还是在gcd()函数定义内部完成为好。写在这里过于突出细节，影响主要思想的表达。

if(temp0&&temp1==1)
printf("%d\n",temp0);
else if(temp0)
printf("%d/%d\n",temp0,temp1);
else
printf("0\n");

　　这段代码显然应该抽象为一个函数，写在这里显得拖泥带水。

return 1;

　　这个奇葩。应该

return 0;

abs()

int abs(int a)
{
return a>0?a:-a;
}

　　这函数的名字显然有问题，不应该取这个名字，因为有一个库函数就是这个名字(声明于stdlib.h中)。当然这样用也不是不可以，但其中涉及到一系列复杂的语言规则。了解这个规则的人虽然知道自己的函数可以与库函数重名，但却从来不会这样用。我相信这位初学者是不知道这个规则的，他(她)这样写是瞎猫碰到死耗子而已。

int gcd(int a,int b)
{
if(a<b)
a^=b^=a^=b;
return b?gcd(b,a%b):a;
}

　　这个函数是错的。

if(a<b)
a^=b^=a^=b;

　　这里有两个问题。第一，a^=b^=a^=b这个表达式是未定义行为(参见，为“a+=a-=a*a”预拟的悼词，“牙里长嘴”和“a+=a-=a*a” )。第二，从后面的代码来看，这里根本就没必要给a、b排序。（参见拙著《品悟C》第五章 画蛇添足 问题12“不彻底的思考”,p152）

续文：分数的加减法——C语言初学者代码中的常见错误与瑕疵(12)