No. of 'a' in a text file

[ume$h]

/* I wrote the following program to calculate no. of 'a' in the file
c:/1.txt but it fails to give appropriate result. What is wrong with
it? */

#include"stdio.h"
int main(void)
{
FILE *f;
char ch;
long int a=0;
f=fopen("c:/1.txt","r");
while(ch=getc(f)!=EOF)
{
switch(ch)
{
case 'a': a++;break;
}
}
printf("No. of 'a' = %d\n",a);
fclose(f);
return 0;
}

 

[Richard Heathfield]

ume$h said:

/* I wrote the following program to calculate no. of 'a' in the file
c:/1.txt but it fails to give appropriate result. What is wrong with
it? */

#include"stdio.h"
int main(void)
{
FILE *f;
char ch;
long int a=0;
f=fopen("c:/1.txt","r");

What happens if the fopen fails?

while(ch=getc(f)!=EOF)

Are you sure you meant to say this? Consider the precedences of = and !=
and check the return type of getc.

{
switch(ch)
{
case 'a': a++;break;

Do you really mean to break out of your loop at this point?

}
}
printf("No. of 'a' = %d\n",a);

Did you read the documentation for printf?

 

[pete]

/* I wrote the following program to calculate no. of 'a' in the file
c:/1.txt but it fails to give appropriate result. What is wrong with
it? */

A lot.

#include"stdio.h"

int main(void)
{
FILE *f;
char ch;

int ch;
/*
** getc returns int, EOF is type int.
*/

long int a=0;
f=fopen("c:/1.txt","r");

What if fopen returns NULL?

while(ch=getc(f)!=EOF)

ch is assigned a value of either 1 or 0,
depending on whether or not getc returns EOF.

{
switch(ch)
{
case 'a': a++;break;
}

You need a default case.

}
printf("No. of 'a' = %d\n",a);
fclose(f);
return 0;
}

/* BEGIN new.c */

#include <stdio.h>

int main(void)
{
FILE *f;
char *fn = "c:/1.txt";
int c;
long unsigned a = 0;

f = fopen(fn,"r");
if (f != NULL) {
while ((c = getc(f)) != EOF) {
a += c == 'a';
}
printf("No. of 'a' = %lu\n", a);
fclose(f);
} else {
printf("fopen problem with %s\n", fn);
}
return 0;
}

/* END new.c */

 

[Umesh]

// This one runs.

#include"stdio.h"
int main(void)
{
FILE *f;
long int a=0;
f=fopen("c:/1.txt","r");
while (getc(f)=='a')
a++;
printf("No. of 'a' = %d\n",a);
fclose(f);
return 0;
}

 

[Richard Tobin]

while(ch=getc(f)!=EOF)

Are you sure you meant to say this? Consider the precedences of = and !=
and check the return type of getc.

{
switch(ch)
{
case 'a': a++;break;

Do you really mean to break out of your loop at this point?[/QUOTE]

What? That does not break out of the loop.

-- Richard

 

[Richard Heathfield]

Umesh said:

// This one runs.

#include"stdio.h"
int main(void)
{
FILE *f;
long int a=0;
f=fopen("c:/1.txt","r");
while (getc(f)=='a')
a++;
printf("No. of 'a' = %d\n",a);
fclose(f);
return 0;
}

On my system, the output of this program is:

"Segmentation fault (core dumped)"

 

[Keith Thompson]

ume$h wrote: [...]

{
switch(ch)
{
case 'a': a++;break;
}

You need a default case.

[...]

What for?

The switch statement is equivalent to:

if (ch == 'a') {
a++;
}

which, of course, would be a better way to write it (unless the OP is
planning to expand it to handle other characters).

 

[Richard Heathfield]

Richard Tobin said:

What? That does not break out of the loop.

You're right, of course. A crit too far.

 

[pete]

// This one runs.

#include"stdio.h"

Should be

int main(void)
{
FILE *f;
long int a=0;
f=fopen("c:/1.txt","r");

What if fopen returns a null pointer?

while (getc(f)=='a')
a++;

This loop will tally the number of 'a' characters
at the begining of the file
and will stop as soon as it reads any other character.

printf("No. of 'a' = %d\n",a);

Should be %ld because (a) is long.

fclose(f);

That function call is undefined if (f) is a null pointer.

 

[pete]

ume$h wrote: [...]

{
switch(ch)
{
case 'a': a++;break;
}

You need a default case.

[...]

What for?

Style?
I forgot that the default case is optional for switch statements.

 

[Keith Thompson]

// This one runs.

#include"stdio.h"
int main(void)
{
FILE *f;
long int a=0;
f=fopen("c:/1.txt","r");
while (getc(f)=='a')
a++;
printf("No. of 'a' = %d\n",a);
fclose(f);
return 0;
}

I'm sure it runs, but it doesn't work. It appears to count the number
of consecutive 'a' characters starting at the beginning of the file,
which I don't think is what you're trying to do.

It also ignores all the advice that's been given to you so far:

Use <stdio.h>, not "stdio.h"

Check the result of fopen().

The "%d" format expects an int; you're giving it a long int.

 

[Umesh]

/*Calculate the no. of occurance of 'ab'. But is this one OK/
EFFICIENT?*/
#include<stdio.h>
int main(void)
{
FILE *f;
long int c,c1,ab=1;
f=fopen("c:/1.txt","r");
while ((c=getc(f))!=EOF && (c1=getc(f))!=EOF) {
if(c=='a' && c1=='b') ab++;}
fclose(f);
printf("No. of 'ab' = %ld\n",ab);
return 0;
}

 

[Richard Heathfield]

Umesh said:

/*Calculate the no. of occurance of 'ab'. But is this one OK/
EFFICIENT?*/
#include<stdio.h>
int main(void)
{
FILE *f;
long int c,c1,ab=1;
f=fopen("c:/1.txt","r");
while ((c=getc(f))!=EOF && (c1=getc(f))!=EOF) {
if(c=='a' && c1=='b') ab++;}
fclose(f);
printf("No. of 'ab' = %ld\n",ab);
return 0;
}

I ran this, and got the following output:

Segmentation fault (core dumped)


But yes, the core dump did happen very quickly. No efficiency complaints
here.

 

[Umesh]

I ran this, and got the following output:

Segmentation fault (core dumped)


But yes, the core dump did happen very quickly. No efficiency complaints
here.

The program is running in TC++ 4.5 and VC++ 6 compiler. What is wrong
with you?
I wanted to say that if I want to find no. of occurance of
'abcdefghijk' in a text file by modifying this program, it would be a
toilsome job. Is there any alternative? Thank you.

 

[attn.steven.kuo]

/*Calculate the no. of occurance of 'ab'. But is this one OK/
EFFICIENT?*/
#include<stdio.h>
int main(void)
{
FILE *f;
long int c,c1,ab=1;

Since getc returns int, I'd
use int for the 'c' and 'c1' variables.

f=fopen("c:/1.txt","r");

Others have said that you
need to check to see if fopen succeeds.

while ((c=getc(f))!=EOF && (c1=getc(f))!=EOF) {
if(c=='a' && c1=='b') ab++;}
fclose(f);
printf("No. of 'ab' = %ld\n",ab);
return 0;

}

And what happens if the file
you're reading contains the
string "bababa\n"? Ask yourself
if the output would be any different
for a file that contained the string
"ababab\n"?

 

[Richard Heathfield]

Umesh said:

The program is running in TC++ 4.5 and VC++ 6 compiler.

No, it isn't. The program doesn't run in the compiler. It runs as a
process on the computer. (One might reasonably describe a C program as
running "in" an interpreter if one happened to be using one, but you
aren't.)

What is wrong with you?

What a question. Here's a better question: what is wrong with your
program, that causes it to produce a segmentation fault on my system
instead of a graceful error message? Hint: fopen can fail.

I wanted to say that if I want to find no. of occurance of
'abcdefghijk' in a text file by modifying this program, it would be a
toilsome job. Is there any alternative? Thank you.

Step 1: make it readable (so that you can correct it).
Step 2: make it correct (so that it's worth speeding up).
Step 3: make it fast (if it isn't already fast enough).

Leaving aside readability issues (although I don't consider your program
to be very readable), you have at least three problems in your program
that stop it from being correct. Firstly, it makes an invalid
assumption that its resource acquisition request is bound to succeed.
Secondly, it starts its count from 1 rather than from 0. And thirdly,
it fails to count any 'a', 'b' pair that are an odd number of bytes
into the file.

I suggest you fix those three problems before worrying about
performance.

 

[Ian Collins]

/*Calculate the no. of occurance of 'ab'. But is this one OK/
EFFICIENT?*/
#include<stdio.h>
int main(void)
{
FILE *f;
long int c,c1,ab=1;
f=fopen("c:/1.txt","r");
while ((c=getc(f))!=EOF && (c1=getc(f))!=EOF) {
if(c=='a' && c1=='b') ab++;}
fclose(f);
printf("No. of 'ab' = %ld\n",ab);
return 0;
}

Whitespace has plummeted in price over the past few years.

Making you program a little safer and readable.

#include <stdio.h>

int main( int argc, char** argv )
{
if( argc > 1 )
{
FILE *f = fopen( argv[1],"r");

if( f )
{
int c,c1;
unsigned ab=0;

while ((c=getc(f))!=EOF && (c1=getc(f))!=EOF)
{
if(c=='a' && c1=='b') ab++;
}

fclose(f);
printf("No. of 'ab' = %ld\n",ab);
}
}
return 0;
}

Now try running it on its self and you find your logic errors.

 

[Richard Heathfield]

Ian Collins said:

Umesh wrote:

Making you program a little safer and readable.

Laudable, but you have introduced at least one fresh bug, by making ab
into an unsigned int...

unsigned ab=0;

printf("No. of 'ab' = %ld\n",ab);

....without fixing the printf to match.

 

[pete]

I wanted to say that if I want to find no. of occurance of
'abcdefghijk' in a text file by modifying this program, it would be a
toilsome job. Is there any alternative? Thank you.

/* BEGIN new.c */

#include <stdio.h>

int main(void)
{
int c;
FILE *f;
char *fn = "c:/1.txt";
long unsigned count = 0;
enum states {A = 0,B,C,D,E,F,G,H,I,J,K} waiting_for = A;

f = fopen(fn,"r");
if (f != NULL) {
while ((c = getc(f)) != EOF) {
switch (c) {
case 'a':
if (waiting_for++ != A) {
waiting_for = A;
}
break;
case 'b':
if (waiting_for++ != B) {
waiting_for = A;
}
break;
case 'c':
if (waiting_for++ != C) {
waiting_for = A;
}
break;
case 'd':
if (waiting_for++ != D) {
waiting_for = A;
}
break;
case 'e':
if (waiting_for++ != E) {
waiting_for = A;
}
break;
case 'f':
if (waiting_for++ != F) {
waiting_for = A;
}
break;
case 'g':
if (waiting_for++ != G) {
waiting_for = A;
}
break;
case 'h':
if (waiting_for++ != H) {
waiting_for = A;
}
break;
case 'i':
if (waiting_for++ != I) {
waiting_for = A;
}
break;
case 'j':
if (waiting_for++ != J) {
waiting_for = A;
}
break;
case 'k':
if (waiting_for == K) {
++count;
}
waiting_for = A;
break;
default:
waiting_for = A;
break;
}
}
printf("No. of 'abcdefghijk' = %lu\n", count);
fclose(f);
} else {
printf("fopen problem with %s\n", fn);
}
return 0;
}

/* END new.c */

 

[pete]

/* BEGIN new.c */

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

#define STRING "abcdefghijk"

int main(void)
{
int c;
FILE *f;
char *letter;
char *fn = "c:/1.txt";
long unsigned count = 0;
enum states {A = 0,B,C,D,E,F,G,H,I,J,K} waiting_for = A;
const char* const string = STRING;

f = fopen(fn, "r");
if (f != NULL) {
while ((c = getc(f)) != EOF) {
switch (c) {
case 'k':
if (waiting_for == K) {
++count;
}
waiting_for = A;
break;
default:
letter = strchr(string, c);
if (letter == NULL
|| waiting_for++ != letter - string)
{
waiting_for = A;
}
break;
}
}
printf("No. of '%s' = %lu\n", string, count);
fclose(f);
} else {
printf("fopen problem with %s\n", fn);
}
return 0;
}

/* END new.c */