Thread: why is duplicate code dangerous?

Code:

 do{
	x = 0;
	do{
	a = fgetc (fp);
	if (a != ' ' && a != '\n' && 
		a != EOF && a !=',' && 
		a !=';' && a !='.'&& 
		a !='!' && a !='?' && 
		a !=':' && a !='`'){
	word[x] = tolower(a);
	  x++;
	  }else{
	if (x != 0){
		word[x] = '\0';
	    t = insertList (word, t);
		wordcount += 1;
		}
	}
      }while (a != ' ' && a != '\n' && 
			  a != EOF && a !=',' && 
			  a !=';' && a !='.'&& 
			  a !='!' && a !='?' && 
			  a !=':' && a !='`');
      
    } while (a != EOF);

here is my function that reads in characters from a .txt file, forms them into words and puts them in a list excluding characters i specified). i was told that this approach is dangerous for some reason and that i should only test characters once. why is that important and how can i rewrite the above loop? (i need the loop to place words in a list while excluding double spaces as well. i tried doing in with one test but double spaces were included into the list and messed up my word count). thanks

Duplicate code is dangerous because if you need to change it, you might forget to change all of the duplicates too. I'd fix your loop with a function.

Code:

int is_punctuation(int a)
{
    return a != ' ' && a != '\n' && 
           a != EOF && a !=',' && 
           a !=';' && a !='.'&& 
           a !='!' && a !='?' && 
           a !=':' && a !='`';
}

instead of doing the same test over and over, call the function.

Code:

do
{
    x = 0;
    do
    {
        a = fgetc (fp);
        if (is_punctuation(a))
        {
            word[x] = tolower(a);
            x++;
        }
        else
        {
            if (x != 0)
            {
                word[x] = '\0';
                t = insertList (word, t);
                wordcount += 1;
            }
        }
    }
    while (is_punctuation(a));
}
while (a != EOF);

I kind of guessed about the name for the function.

thanks i will try that.

under <ctype.h> isnt there a ispunc command?

Originally Posted by agentsmith

Code:

 do{
	x = 0;
	do{
	a = fgetc (fp);
	if (a != ' ' && a != '\n' && 
		a != EOF && a !=',' && 
		a !=';' && a !='.'&& 
		a !='!' && a !='?' && 
		a !=':' && a !='`'){
	word[x] = tolower(a);
	  x++;
	  }else{
	if (x != 0){
		word[x] = '\0';
	    t = insertList (word, t);
		wordcount += 1;
		}
	}
      }while (a != ' ' && a != '\n' && 
			  a != EOF && a !=',' && 
			  a !=';' && a !='.'&& 
			  a !='!' && a !='?' && 
			  a !=':' && a !='`');
      
    } while (a != EOF);

here is my function that reads in characters from a .txt file, forms them into words and puts them in a list excluding characters i specified). i was told that this approach is dangerous for some reason and that i should only test characters once. why is that important and how can i rewrite the above loop? (i need the loop to place words in a list while excluding double spaces as well. i tried doing in with one test but double spaces were included into the list and messed up my word count). thanks

That looks a bit messy to me.
I would do it something like this, if I have understood the question correctly.
It's just the general idea and it has not been compiled or tested.
So don't blame me if it does not work or compile :O)
I would be surprised if it did!!
Well not that surprised ;O)

Code:

char word[MAX_NUMBER_OF_WORDS][MAX_WORD_LENGTH];

int w=0;
int  x=0;
while ( ( a = fgetc (fp)   ) != EOF){
       v= tolower(a);
       if (v>='a' && v<='z'){
            word[w,x++] = a;       
       }
        else{
	        w++; x=0;
	}
 }

Originally Posted by vart

But of course we will blame you. Somebody posting the example should AT LEAST learn the syntax of the language

Not really, the compiler is better at that then me, it will inform me of any errors
when I compile it.

But then I expect all the code you write compiles first time because you are so clever???

Originally Posted by vart

But of course we will blame you. Somebody posting the example should AT LEAST learn the syntax of the language

OK so I put
word[w,x++] = a;
Instead of
word[w][x++] = a;

Big deal!
Seems to compile nicely now, and no doubt work ;O)

Code:

int word[MAX_NUMBER_OF_WORDS][MAX_WORD_LENGTH];

int w=0;
int  x=0;
while ( ( a = fgetc (fp)   ) != EOF){
       v= tolower(a);
       if (v>='a' && v<='z'){
            word[w][x++] = a;       
       }
        else{
	        w++; x=0;
	}
 }