劣质代码评析——《写给大家看的C语言书(第2版)》附录B之21点程序(一)
《写给大家看的C语言书(第2版)》是邮电社图灵公司引进翻译的一本C语言入门书,这是一本垃圾书。搞不清图灵为什么引进了这样一本垃圾书。该书作者基本不懂得C编程技术,书中误导、错谬比比皆是。
该书的附录B给出了一个21点游戏的代码,这是一个很糟糕的C程序,毛病很多,实在不足以为初学者以示范,当反面教材还差不多。考虑到其中的很多毛病初学者也有,所以拿来在此评析一番。
首先,这段程序可读性极差,甚至可以说是乱作一团。连我这种阅读代码能力很强的人看起来都很费劲,初学者根本就没有可能看懂。
乱作一团和可读性还不是一回事。可读性指的是代码晦涩难懂,乱作一团则是指不顾起码的编程规范。譬如代码没有基本的缩进
do { ans = getAns("Hit or stand (H/S)?"); if ( ans = 'H' ) { playerGetsCard(&numCards,cards, playerPoints); } }while(ans != 'S' );
任意地截断语句或声明
do { initCardsScreen(cards,playerPoints,dealerPoints, total, &numCards); dealerGetsCard(&numCards,cards, dealerPoints);
代码本来就够乱的了,又塞进了一大堆更乱更不规范的注释。这些注释其实是应该翻译过来的,但是很可惜译者偷懒并没有翻译,对于国内初学者来说,这就更加混乱。混乱的代码和混乱的注释纠结在一起,整个源程序惨不忍睹。
为了不折磨大家眼球,下面以删除了注释的代码为基础评析。大家也可以顺便比较一下(http://ishare.iask.sina.com.cn/f/20886221.html),看看这些注释是不是起了很坏的作用。如果感觉这里删除了注释的代码更清楚一些,那么无疑原书上的注释是垃圾。
#include <stdio.h> #include <time.h> #include <ctype.h> #include <stdlib.h> #define BELL '\a' #define DEALER 0 #define PLAYER 1 #define ACELOW 0 #define ACEHIGH 1 int askedForName = 0; void dispTitle(void); void initCardsScreen(int cards[52],int playerPoints[2], int dealerPoints[2], int total[2], int *numCards); int dealCard(int * numCards,int cards[52]); void dispCard(int cardDrawn,int points[2]); void totalIt(int points[2],int tatal[2],int who); void dealerGetsCard(int *numCards,int cards[52], int dealerPoints[2]); void playerGetsCard(int *numCards,int cards[52], int playerPoints[2]); char getAns(char mesg[]); void findWinner(int total[2]); main() { int numCards; int cards[52],playerPoints[2],dealerPoints[2],total[2]; char ans; do { initCardsScreen(cards,playerPoints,dealerPoints, total, &numCards); dealerGetsCard(&numCards,cards, dealerPoints); printf("\n"); playerGetsCard(&numCards,cards,playerPoints); playerGetsCard(&numCards,cards,playerPoints); do { ans = getAns("Hit or stand (H/S)?"); if ( ans = 'H' ) { playerGetsCard(&numCards,cards, playerPoints); } }while(ans != 'S' ); totalIt(playerPoints,total,PLAYER); do{ dealerGetsCard(&numCards,cards,dealerPoints); }while (dealerPoints[ACEHIGH] < 17 ); totalIt(dealerPoints,total,DEALER); findWinner(total); ans=getAns("\nPlay again(Y/N)?"); }while(ans=='Y'); return ; } void initCardsScreen(int cards[52],int playerPoints[2], int dealerPoints[2], int total[2], int *numCards) { int sub,val = 1 ; char firstName[15]; *numCards=52; for(sub=0;sub<=51;sub++){ val = (val == 14) ? 1 : val; cards[sub] = val; val++; } for(sub=0;sub<=1;sub++) { playerPoints[sub]=dealerPoints[sub]=total[sub]=0;} dispTitle(); if (askedForName==0) { printf("What is your first name?"); scanf(" %s",firstName); askedForName=1; printf("Ok, %s,get ready for casino action!\n\n", firstName); getchar(); } return; } void playerGetsCard(int *numCards,int cards[52], int playerPoints[2]) { int newCard; newCard = dealCard(numCards, cards); printf("You draw:"); dispCard(newCard,playerPoints); } void dealerGetsCard(int *numCards,int cards[52], int dealerPoints[2]) { int newCard; newCard = dealCard(numCards,cards); printf("The dealer draws:"); dispCard(newCard,dealerPoints); } int dealCard(int * numCards,int cards[52]) { int cardDrawn,subDraw; time_t t; srand(time(&t)); subDraw = (rand()%(*numCards)); cardDrawn = cards[subDraw]; cards[subDraw] = cards[*numCards -1]; (*numCards)-; return cardDrawn; } void dispCard(int cardDrawn, int points[2]) { switch(cardDrawn){ case(11): printf("%s\n","Jack"); points[ACELOW] += 10; points[ACEHIGH] += 10; break; case(12): printf("%s\n","Queen"); points[ACELOW] += 10; points[ACEHIGH] += 10; break; case(13): printf("%s\n","King"); points[ACELOW] += 10; points[ACEHIGH] += 10; break; default : points[ACELOW] += cardDrawn; if(cardDrawn==1) { printf("%s\n","Ace"); points[ACEHIGH]+= 11; } else { points[ACEHIGH]+=cardDrawn; printf("%d\n",cardDrawn); } } return ; } void totalIt(int points[2],int total[2],int who) { if ((points[ACELOW] == points[ACEHIGH])|| (points[ACEHIGH] > 21 )) { total[who] = points[ACELOW];} else { total[who] = points[ACEHIGH];} if (who == PLAYER ) {printf("You have a total of %d\n\n", total[PLAYER]);} else {printf("The house stands with a total of %d\n\n", total[DEALER]);} return; } void findWinner(int total[2]) { if ( total[DEALER] == 21 ) { printf("The house wins.\n"); return ;} if ( (total[DEALER] > 21) && (total[PLAYER] > 21)) { printf("%s", "Nobody wins.\n"); return ; } if ((total[DEALER] >= total[PLAYER])&& (total[DEALER] < 21)) { printf("The house wins.\n"); return ; } printf("%s%c","You win!\n",BELL); return; } char getAns(char mesg[]) { char ans; printf("%s", mesg); ans = getchar(); getchar(); return toupper(ans); } void dispTitle(void) { int i = 0 ; while(i<25) { printf("\n"); i++; } printf("\n\n*Step right up to the Blackjack tables*\n\n"); return ; }
然而尽管去除了脏、乱、差的注释,代码依然很糟糕,例如
{ printf("\n"); i++; }
更拙劣的是这个
{ points[ACEHIGH]+=cardDrawn; printf("%d\n",cardDrawn); } }
注意到中间有个小三(“}”)吗?这风格简直丑疯了,只好再整理一下。
#include <stdio.h> #include <time.h> #include <ctype.h> #include <stdlib.h> #define BELL '\a' #define DEALER 0 #define PLAYER 1 #define ACELOW 0 #define ACEHIGH 1 int askedForName = 0; void dispTitle(void); void initCardsScreen(int cards[52],int playerPoints[2], int dealerPoints[2], int total[2], int *numCards); int dealCard(int * numCards,int cards[52]); void dispCard(int cardDrawn,int points[2]); void totalIt(int points[2],int tatal[2],int who); void dealerGetsCard(int *numCards,int cards[52], int dealerPoints[2]); void playerGetsCard(int *numCards,int cards[52], int playerPoints[2]); char getAns(char mesg[]); void findWinner(int total[2]); main() { int numCards; int cards[52],playerPoints[2],dealerPoints[2],total[2]; char ans; do { initCardsScreen(cards,playerPoints,dealerPoints,total, &numCards); dealerGetsCard(&numCards,cards, dealerPoints); printf("\n"); playerGetsCard(&numCards,cards,playerPoints); playerGetsCard(&numCards,cards,playerPoints); do { ans = getAns("Hit or stand (H/S)?"); if ( ans == 'H' ) { playerGetsCard(&numCards,cards,playerPoints); } } while( ans != 'S' ); totalIt(playerPoints,total,PLAYER); do { dealerGetsCard(&numCards,cards,dealerPoints); } while (dealerPoints[ACEHIGH] < 17 ); totalIt(dealerPoints,total,DEALER); findWinner(total); ans = getAns("\nPlay again(Y/N)?"); } while(ans=='Y'); return ; } void initCardsScreen( int cards[52],int playerPoints[2], int dealerPoints[2], int total[2], int *numCards ) { int sub,val = 1 ; char firstName[15]; *numCards=52; for(sub=0;sub<=51;sub++) { val = (val == 14) ? 1 : val; cards[sub] = val; val++; } for(sub=0;sub<=1;sub++) { playerPoints[sub]=dealerPoints[sub]=total[sub]=0; } dispTitle(); if (askedForName==0) { printf("What is your first name?"); scanf(" %s",firstName); askedForName=1; printf("Ok, %s,get ready for casino action!\n\n",firstName); getchar(); } return; } void playerGetsCard(int *numCards,int cards[52],int playerPoints[2]) { int newCard; newCard = dealCard(numCards, cards); printf("You draw:"); dispCard(newCard,playerPoints); } void dealerGetsCard(int *numCards,int cards[52],int dealerPoints[2]) { int newCard; newCard = dealCard(numCards,cards); printf("The dealer draws:"); dispCard(newCard,dealerPoints); } int dealCard(int * numCards,int cards[52]) { int cardDrawn,subDraw; time_t t; srand(time(&t)); subDraw = (rand()%(*numCards)); cardDrawn = cards[subDraw]; cards[subDraw] = cards[*numCards -1]; (*numCards)-; return cardDrawn; } void dispCard(int cardDrawn, int points[2]) { switch(cardDrawn) { case(11): printf("%s\n","Jack"); points[ACELOW] += 10; points[ACEHIGH] += 10; break; case(12): printf("%s\n","Queen"); points[ACELOW] += 10; points[ACEHIGH] += 10; break; case(13): printf("%s\n","King"); points[ACELOW] += 10; points[ACEHIGH] += 10; break; default : points[ACELOW] += cardDrawn; if(cardDrawn==1) { printf("%s\n","Ace"); points[ACEHIGH]+= 11; } else { points[ACEHIGH]+=cardDrawn; printf("%d\n",cardDrawn); } } return ; } void totalIt(int points[2],int total[2],int who) { if ( (points[ACELOW] == points[ACEHIGH]) ||(points[ACEHIGH] > 21 )) { total[who] = points[ACELOW]; } else { total[who] = points[ACEHIGH]; } if (who == PLAYER ) { printf("You have a total of %d\n\n", total[PLAYER]); } else { printf("The house stands with a total of %d\n\n", total[DEALER]); } return; } void findWinner(int total[2]) { if ( total[DEALER] == 21 ) { printf("The house wins.\n"); return ; } if ( (total[DEALER] > 21) && (total[PLAYER] > 21) ) { printf("%s", "Nobody wins.\n"); return ; } if ((total[DEALER] >= total[PLAYER])&& (total[DEALER] < 21)) { printf("The house wins.\n"); return ; } printf("%s%c","You win!\n",BELL); return; } char getAns(char mesg[]) { char ans; printf("%s", mesg); ans = getchar(); getchar(); return toupper(ans); } void dispTitle(void) { int i = 0 ; while(i<25) { printf("\n"); i++; } printf("\n\n*Step right up to the Blackjack tables*\n\n"); return ; }
这回勉强能看得下去了。现在不难发现main()中
return ;
实际上是
return 0;
之误。
在dealCard()函数中,也存在一个明显的错误
(*numCards)-;
实际上应该是
(*numCards)--;
或
--*numCards;
现在来阅读代码。首先看到的是
int askedForName = 0;
这个外部变量看起来很别扭。从21点这个问题来看,怎么也不会想到程序会需要这样一个名为askedForName 的变量,对于整个问题来讲,这个变量是无关痛痒的鸡毛蒜皮。把这种鸡毛蒜皮性质的变量提升到外部变量这样重要的代码地位,就如同把家里一件可能几十年还用不到一次的小东西布置在客厅显眼位置一样荒唐。
那么作者为什么要设置这个外部变量呢?估计有两种可能,一种是缺乏足够的代码写作技能,某段代码实在写不下去了,无可奈何地用了一个外部变量。另一种情况是写代码之前缺乏充分缜密的构思,思路有漏洞,写到中间时才发现,只好用这种办法来补救。后一种情况比前一种更糟糕。这就像盖房子一样,盖到中间才发现继续盖下去,房子的一个墙角就会盖到水塘里一样,之后匆忙修改图纸,房子无法按原计划形状盖成只好不顾了。
实际上这时应该重新思考,重写代码,而不是修修补补。
(未完待续)