Code review

R

Ravi Uday

Hi,
This code finds the number of non-commented lines in a given source
file.
Works fine except for one condition that i found.
If the Input file is of the below kind :
#include<xyz.h>/* declare headers
declare main*/int main()
{
return 0;
}
/* Input file ends here */

Result:
actual output should be : Number of non-commented lines is 5
Output from code I got is : Number of non-commented lines is 4 !

Can anyone of you point the mistake or better way of going about.
- Ravi


#include <stdio.h>
#include <string.h>
#include <stdlib.h>

/* Maximum chars in any line. If this is crossed then the remaining
bytes are ignored */
#define BYTES 512

/* line is valid if it contains any of the following chars. Otherwise
treated as commented */
#define VALID_CHARS "\\abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ;,'/#{}()*+-0123456789"

int comment_handler ( FILE *fptr, char *c)
{
int ind = 0;
char ch;

if ( ( ch = fgetc ( fptr )) =='*')/* checks for '/*' */
{
while ( !ind )
{
if (fgetc ( fptr ) == '*' )
{
if ( fgetc ( fptr ) == '/')/* checks for end of comment */
{
ind = 1;
*c = fgetc ( fptr);
}
}
}
}
else if ( ch == '/')/* checks cpp comment */
{
while ( (!feof (fptr) ) && ( ch != '\n'))/* end of line check for \n
*/
ch = fgetc ( fptr );

*c = ch;
}
else/* Special case: No comments found */
{
*c = ch;/* Storing next character */
return 0;/* The char is single '/' */
}

return 1;
}

int main (int argc, char *argv[])
{
int i = 0, j = 0, flag = 0;
char ch = 0;
char buffer[BYTES];/* variable holds a max of BYTES (defined) chars in
any line */
FILE *fp = NULL, *fout = NULL;

if ( argc < 3 ) return EXIT_FAILURE;

fp = fopen ( argv[1], "r+b");/* open the source file */

if ( fp == NULL ) return EXIT_FAILURE;

fflush ( stdout);
fout = fopen ( argv[2], "w+b");/* open the output file */

if ( fout == NULL ) return EXIT_FAILURE;

while ( (!feof (fp )) && (!ferror (fp) ))
{
memset ( buffer, 0x00, BYTES);
while ( (j != BYTES-1) && (!flag) )/* Check for max BYTES-1 chars */
{
ch = fgetc ( fp );

if ( ch == '/')
if (comment_handler ( fp, &ch ) == 0)
buffer[j++] = '/';

if ( (ch == '\n') || (feof (fp)))
flag = 1;

/* dont add '\n' or '\r' to the running buffer cause its appended in
the fprintf */
if ( ( ch !='\n' ) && ( ch != '\r'))
buffer[j++] = ch;
}
if (j == BYTES-1)/* line has more than BYTES chars, so ignore them !
*/
{
j++;
while ( (!feof (fp )) && (!ferror (fp) ))
{
ch = fgetc(fp);
if ( ch == '\n')
break;
}
}

if (strpbrk (buffer, VALID_CHARS))
{
fprintf ( fout, "%s\n", buffer);
i++;
}

j = flag = ch = 0;
}
printf ("\n*** Number of non commented lines is : %d ***\n\n", i);

fclose ( fp );
fclose ( fout );
return EXIT_SUCCESS;
}
 
M

Martien Verbruggen

Hi,
This code finds the number of non-commented lines in a given source
file.
Works fine except for one condition that i found.

It also fails when there is a 'comment' inside of a string:

char *some_string = "A string /* with a comment */";
Can anyone of you point the mistake or better way of going about.

Use a state machine, and parse character by character, so that at all
times you know whether you're in a comment or not, and whether you're
in a string or not.

A few remarks:
/* Maximum chars in any line. If this is crossed then the remaining
bytes are ignored */
#define BYTES 512

You should at least put out a warning when you ignore that sort of
thing.
if ( ( ch = fgetc ( fptr )) =='*')/* checks for '/*' */

This is an illegal comment.
if ( argc < 3 ) return EXIT_FAILURE;

You should print out a usage statement. Especially if you expect
people to try your code.
fp = fopen ( argv[1], "r+b");/* open the source file */

You keep talking about "lines" but here you open the file in binary
mode. And why do you open it for update?
if ( fp == NULL ) return EXIT_FAILURE;

You should print an error message.
fflush ( stdout);
Why?

fout = fopen ( argv[2], "w+b");/* open the output file */

Again, why open for update, when you intend to just write?
while ( (!feof (fp )) && (!ferror (fp) ))

You should read the C FAQ (http://www.eskimo.com/~scs/C-faq/top.html),
question 12.2. This is not the right way to read from a file.

[snip of very convoluted loop block]


Martien
 
G

Gordon Burditt

Hi,
This code finds the number of non-commented lines in a given source
file.
Works fine except for one condition that i found.
If the Input file is of the below kind :
#include<xyz.h>/* declare headers GOT COMMENT
declare main*/int main() GOT COMMENT
{ NOT GOT COMMENT 1
return 0; NOT GOT COMMENT 2
} NOT GOT COMMENT 3
/* Input file ends here */ GOT COMMENT (if this line was even included at all)

Result:
actual output should be : Number of non-commented lines is 5
Output from code I got is : Number of non-commented lines is 4 !

I don't agree with your correct answer.
I claim the correct number of non-commented lines is 3.

Perhaps you would care to define "non-commented line" and explain
how you got the so-called "correct answer"?

"a non-commented line is a line that has no part of a comment in it".

Gordon L. Burditt
 
J

Julian V. Noble

Ravi said:
[ deleted ]
Use a state machine, and parse character by character, so that at all
times you know whether you're in a comment or not, and whether you're
in a string or not.

Any sites which mention of State machines and ways of doing that ?
As for comment in a string i shall write another function which makes sure
that if a * " * is found then parse till it finds another * " * and treat it
as a non-commented line.

http://www.jfar.org/article001.html


--
Julian V. Noble
Professor Emeritus of Physics
(e-mail address removed)
^^^^^^^^^^^^^^^^^^
http://galileo.phys.virginia.edu/~jvn/

"Science knows only one commandment: contribute to science."
-- Bertolt Brecht, "Galileo".
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads


Members online

Forum statistics

Threads
473,780
Messages
2,569,611
Members
45,264
Latest member
FletcherDa

Latest Threads

Top