分数的加减法——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）
