# Delete all comments in a C program

I am new to C. And I just want to know if my code is horrible or not. I tried to make program to delete all comments in C program. It's from "The C Programming Language Second Edition" book, Exercise 1-23.

Here is my solution :

#include <stdio.h>
#define MAXCHAR 1000
int c = 0;
int d = 0;
int newFileText[MAXCHAR];
int longDisable = 0;
int shortDisable = 0;
int i = 0;

//Function declarations
void printArray(int a[], int b);

void myputchar(int a[], int c);

int main(void) {
while ((c = getchar()) != EOF) {
if (i > MAXCHAR)i = 0;
if (c == '/') {
d = getchar();
if (d == '/') shortDisable = 1;
else if (d == '*') longDisable = 1;
} else if (c == '*') {
d = getchar();
if (d == '/') {
c = getchar();
if (!(c > 0)) {
break;
}
longDisable = 0;
}
} else if (c == '\n') {
shortDisable = 0;
}
if (shortDisable || longDisable);
else {
myputchar(newFileText, c);
i++;
}
/*printf("\nShort disable: %d", shortDisable);
printf("\nLong disable: %d", longDisable);*/
}
printArray(newFileText, i);
return 0;
}

void printArray(int theArray[], int lengthSoFar) {
for (int i = 0; i < lengthSoFar; i++) {
printf("%c", theArray[i]);
}
}

void myputchar(int a[], int c) {
int i = 0;
while (a[i] != '\0') {
i++;
}
a[i] = c;
}

• Take a look at this other code review which tried to solve the same exercise. Maybe you can learn something from that question and it's answers, too. – I'll add comments tomorrow Sep 29 '16 at 18:12
• Its far too complicated than it should be i think.There is more simpler answer.Answered by @200_success – Muhamed Cicak Sep 29 '16 at 18:35
• But ivan's solution accounts for string literals correctly, so there is something to be learned from it. – 200_success Sep 29 '16 at 18:37
• I'm just too beginner to C, to understand his code.I had a look.But nothing seems understandable for me :) – Muhamed Cicak Sep 29 '16 at 18:38
• Well ^that being said, you have to start learning new things. It's useful. Enjoy your trip in C-world ! – Grajdeanu Alex. Sep 30 '16 at 4:18

## Bugs

When processing an expression like 10/2 or 10*2, the 2 gets discarded from the output.

It looks like you attempted to implement a circular buffer for the output, but it actually fails miserably if the output exceeds 1000 bytes. One problem is performance: myputchar() performs a linear scan of the buffer to find the end of the string, where it can write the next character. A second problem is buffer overflow: myputchar() makes no attempt to ensure that the location where it stores the next character lies within the buffer. The third problem is that when you do if (i > MAXCHAR)i = 0, you haven't made any attempt to print the buffer contents before trying to reuse the buffer.

## C grammar

In C, a /* or // that appears within a "string literal" should be treated literally. It should not be interpreted as a start-of-comment marker.

Furthermore, within a //-style comment, any /* or */ needs to be disregarded. Similarly, within a /* */-style comment, any // should be disregarded. That is, the correct output for the following C99 program…

#include <stdio.h>
int main() {
puts("First " /* asdf // */ "example"
);
puts("Second " // /*
"example"
// */
);
puts("Third " /* "/" */ "example");
}


… should be

First example
Second example
Third example


(I also got this wrong in Rev 1 of this answer. I've corrected the bug, but this illustrates a point that C requires a stateful parser.)

## Organization

The variables c, d, newFileText, longDisable, shortDisable, and i are global; they should all be local to the main function.

It is customary to put the main function last, so that you don't need to declare the functions it uses.

## Input / output

You read one character at a time with getchar(), but you attempt to print the output in chunks of up to 1000 characters. Writing in chunks could be more efficient than writing a character at a time, but it is certainly more difficult to implement correctly. However, after you go through the effort of accumulating the chunks, you then proceed to print the buffer one character at a time using printf("%c", theArray[i]), which negates any performance advantage you could have gained.

Therefore, you would have been better off printing the output one character at a time to begin with, without any attempt at buffering. To do so, you could use putchar(…), which is simpler than printf("%c", …).

If you did want to efficiently write a chunk of bytes at a time, you could use the write() function instead of the loop in printArray().

## Suggested solution

This solution addresses the more serious logic bugs, without attempting to handle string literals correctly.

#include <stdio.h>

int main(void) {
int c, d;
int longDisable = 0, shortDisable = 0;

while ((c = getchar()) != EOF) {
switch (c) {
case '/':
case '*':
d = getchar();
if (!longDisable && c == '/' && d == '/') {
shortDisable = 1;
} else if (!shortDisable && c == '/' && d == '*') {
longDisable = 1;
} else if (!shortDisable && c == '*' && d == '/') {
longDisable = 0;
} else if (!(shortDisable || longDisable)) {
putchar(c);
if (d != EOF) putchar(d);
}
break;

case '\n':
shortDisable = 0;
/* fall through... */
default:
if (!(shortDisable || longDisable)) {
putchar(c);
}
}
}
}

• Thank you for the lesson :) And some people told me that (char c=getchar())!=EOF is bad to do.They said i should do int c instead of char c. – Muhamed Cicak Sep 29 '16 at 18:22
• Actually, you're right. Technically, it should be int c, d. – 200_success Sep 29 '16 at 18:23
• And there is one part that i dont understand very well.I am not really familiar with switch statement so i might be wrong.You wrote if (c=='/' && d=='/').Isnt c anyways equal to '*' there ?? Because its in case '*' ?? – Muhamed Cicak Sep 29 '16 at 18:29
• No. The way switch works, if c is either '/' or '*', the code starting with d = getchar() will be executed. – 200_success Sep 29 '16 at 18:34
• Oh,okay i understand now.Thank you for helping me out.This code looks much better than mine.Its far too simpler.Nice lesson :) – Muhamed Cicak Sep 29 '16 at 18:36