Previous link: http://www.cnblogs.com/pmer/archive/2012/12/13/2817180.html#sample]
[Evaluation]
The design of the construction side is not counted, but the "assembly" is the final consideration ". This is a typical "bottom-up" rather than the "top-down" advocated by the unstructured program design ". On the one hand, the author exaggerated what "in the C language programming," top-down, gradually refinement "is like a magic spell, as long as we read this spell, any problems with responsibility will be solved (P40). On the other hand, the "top-down" principle will be trampled upon during coding. This is a typical anti-red flag.
[Sample]
[Evaluation]
Since Dijkstra pointed out that GOTO is harmful, this kind of mouse nest flow chart is rare. Everybody, hurry up and look at the mouse nest.
How do you think it's like digging a big hole, then jumping down and burying yourself, then you have to dig another hole to drill it out.
[Sample]
[Evaluation]
With such a flowchart, it is not surprising that the code is chaotic and complex.
First
while(true) { /*……*/ if(strlen(wd)>0) { /*……*/ } else { break; } }
This structure is very bad, so it is better to write
while(true) { /*……*/ if(strlen(wd)>0) { /*……*/ continue; } break; }
The most important reason for writing such a bad structure is that the author does not know how to define variables, and defines the char [30] of WD in the loop body of the while statement, however, this while statement is used to process WD. This reminds us of a joke. A stupid mother-in-law stitched herself into the quilt. This is the case with WD. This should be defined outside the while statement, but it is "sewed" in the loop body. Finally, I had to use the last trick, "break", just like my stupid mother-in-law.
If we define WD outside the while statement, the while statement is much simpler.
char wd[30] ; while( text = cutword(text,wd) , strlen(wd)>0 ) { file->total +=1; word* exist = findnode(file->list,wd); if( NULL == exist) { word* node = createnode(wd); if(NULL == pre) { file->list = node; } else { pre->next = node; } pre = node; } else { exist->count +=1; } }
Strlen (WD)> 0 in the sample is also a poor method, because it is equivalent to * WD! = '\ 0 '. The callback function is called in each loop and only once in each loop. "! = "Computing, efficiency is obviously a world of difference.
The null = pre in the sample is logically incorrect and should be written as null = file-> list.
In addition
if(NULL == pre) { file->list = node; } else { pre->next = node; } pre = node;
Is a very clumsy way of writing. This method separates the head of the linked list from the next member of the node. Here, you must write an IF-else statement. The simpler way of writing is to regard the two as the same thing, so that they can be treated in a unified manner. Another reason for this awkwardness is the rigid and stubborn addition of the new node to the end of the linked list. In fact, this is not necessary.
To sum up, this function can be better written
void parseword(txtfile* file){ char* text = file->text; file->list = NULL; file->total = 0; char wd[30] ; cleantext(text); while( text = cutword(text,wd) , *wd != '\0' ) { file->total +=1; word* exist = findnode(file->list,wd); if( NULL == exist) { word* node = createnode(wd); node->next = file->list; file->list = node ; } else { exist->count +=1; } }}
Of course, there are other problems in the code, but these problems are related to the overall thinking of the program or other functions, which cannot be further corrected here.