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

#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显然是operantion的缩写。

 

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）