having a problem with calling a function

[interpim]

Im trying to teach myself programming, and I am doing exercises out of the book C by Dissection by Ira Pohl.
I am stuck on one of them and can't figure out how to fix it.

The exercise is for you to write a program that asks for a user inputted number and then print out that many prime numbers. The function that determines if a number is prime or not is provided.
My problem is that I can't figure out how to call the function repetitively and only print the numbers that return a true from the prime number function. What I have so far is this.

#include "primes.h"

main(void)
{
int x,y,n;

printf("PRIME NUMBERS WILL BE PRINTED\n");
printf("How many do you want to see? ");
scanf("%d",&x);
is_prime(n);
{
for(y=0;y<=x;++y)
printf("%5d: %5d\n",y,n)
}
}



here is is_prime.c

#include "primes.h"

int is_prime(int n)
{
int k, limit;

if (n == 2)
return 1;
if (n % 2 == 0)
return 0;
limit = n / 2;
for (k = 3; k <= limit; k += 2)
if (n % k == 0)
return 0;
return 1;
}


here is primes.h

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

int is_prime(int n);


Please help me.... this thing is really bugging the crap out of me.

 

[Richard Heathfield]

Im trying to teach myself programming, and I am doing exercises out of the
book C by Dissection by Ira Pohl. I am stuck on one of them and can't
figure out how to fix it.

The exercise is for you to write a program that asks for a user inputted
number and then print out that many prime numbers. The function that
determines if a number is prime or not is provided.

That function returns a value. Use it. (See below.)

My problem is that I can't figure out how to call the function
repetitively and only print the numbers that return a true from the prime
number function. What I have so far is this.

#include "primes.h"

You forgot:

#include <stdio.h>

This is required, because you are using printf.

main(void)
{
int x,y,n;

printf("PRIME NUMBERS WILL BE PRINTED\n");
printf("How many do you want to see? ");
scanf("%d",&x);
is_prime(n);
{
for(y=0;y<=x;++y)
printf("%5d: %5d\n",y,n)
}

Your is_prime function doesn't work correctly for an input value of 1, so
change this to:

for(y = 2; y <= x; ++y)
{
if(is_prime(n))
{
printf("%d is prime\n", y);
}
else
{
printf("%d is composite.\n", y);
}
}

Now, main() returns int, so return an int from main():

return 0;

 

[David]

Does this make sense?

main(void)
{
int x,y,n;

printf("PRIME NUMBERS WILL BE PRINTED\n");
printf("How many do you want to see? ");
scanf("%d",&x);
for(y=0;y<=x;++y)
{
n=is_prime(y);
printf("%5d: %5d\n",y,n);
}
}

David

 

[Kevin Goodsell]

#include "primes.h"

main(void)

Make this

int main(void)

"Implicit int" has been removed from C.

{
int x,y,n;

printf("PRIME NUMBERS WILL BE PRINTED\n");
printf("How many do you want to see? ");

If you want to be sure the user will see your prompt, you need to either
output a newline or flush the stream with fflush(stdout).

scanf("%d",&x);
is_prime(n);

Here you call a function. That function's purpose is to take an argument
and return a result based on that argument. Ask yourself 2 questions: 1)
What argument have you given it? 2) What have you done with the result?

The answers are that you have given it a garbage (indeterminate) value,
and thrown away the result. This is clearly not a useful thing to do.

{

This is a spurious bracket. There seems to be no reason for it.

for(y=0;y<=x;++y)

How many times will this loop be executed? How many times did you want
it to be executed?

printf("%5d: %5d\n",y,n)

At this point n still does not have a useful value, so you will probably
print garbage (though technically the behavior is undefined, so there
are no restrictions on what may happen).

}
}

What you really want to do is:

1. Read in a number, n.
2. Create a counter, initialize to 0.
3. Iterate through integers, testing if each is a prime.
4. When a prime is found, print it and increment the counter.
5. Once the counter reaches n, terminate.

You obviously need to call is_prime() inside the loop, and decide
whether or not to print & increment the counter based on the result of
is_prime. Your loop might look something like this:

for (count=0, x=0; count<n; ++x)
{
if (is_prime(x))
{
/* print number, increment count */
}
}

I'm using the variable names differently than in your code - n is the
number of primes to print and x iterates through the integers.

here is primes.h

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

There doesn't seem to be a reason to include these here.

int is_prime(int n);

-Kevin

 

[Dan Pop]

You forgot:

#include <stdio.h>

Nope, he hasn't. It's in "primes.h", which you have snipped without
even reading.

Dan

 

[interpim]

I appreciate you guys helping me out.... I haven't tested it yet but the way Kevin layed it out, makes it easier for me to understand... Thanks again.

 

[Christopher Benson-Manica]

Im trying to teach myself programming, and I am doing exercises out of the book C by Dissection by Ira Pohl.

main(void)

No one's mentioned it yet (that I noticed), but if this is how your
book declares main(), you might be well advised to choose a different
book. "The C Programming Language" (second edition, by Kernighan and
Ritchie) would be an excellent choice.

 

[Alex]

interpim <[email protected]> spoke thus:

No one's mentioned it yet (that I noticed), but if this is how your
book declares main(), you might be well advised to choose a different
book. "The C Programming Language" (second edition, by Kernighan and
Ritchie) would be an excellent choice.

There is nothing wrong with that declaration of main with respect
to C89. In fact, I recall the following being used throughout K&R2:

main()
{

}

.... which is worse than the OP's example when 'main' may be invoked
recursively.

Alex

 

[Default User]

No one's mentioned it yet (that I noticed), but if this is how your
book declares main(), you might be well advised to choose a different
book. "The C Programming Language" (second edition, by Kernighan and
Ritchie) would be an excellent choice.

Really? Here's K&R's first program:


In C, the program to print "hello, world" is

#include <stdio.h>

main()
{
printf("hello, world\n");
}



Brian Rodenborn

 

[Keith Thompson]

In <[email protected]> Richard Heathfield


Nope, he hasn't. It's in "primes.h", which you have snipped without
even reading.

Fair enough -- but as a matter of style, it's better to provide a
"#include <stdio.h>" directive in each source file that needs it
(especially since neither "primes.h" nor "primes.c" depends on
anything in <stdio.h>).

 

[Christopher Benson-Manica]

In C, the program to print "hello, world" is
main()

Can I claim that main() is bad style, at least? Or should I just
open my mouth and insert a foot or two?

Sorry ;-(

 

[Default User]

Can I claim that main() is bad style, at least? Or should I just
open my mouth and insert a foot or two?

The point was, of course, that just because a book uses some things that
became obsolete with the new standard doesn't invalidate the book.

Now, the use of

void main()

Does cast doubt on a book's value.





Brian Rodenborn

 

[Dave Vandervies]

Can I claim that main() is bad style, at least? Or should I just
open my mouth and insert a foot or two?

Sorry ;-(

Yes, you can claim that "main()" is bad style. Inserting a foot might
not be a bad idea either; even if it's not appropriate here, getting
lots of practice prying them back out is usually a Good Thing.

K&R do it because it wasn't yet established as bad style when they wrote
their book. (It was the canonical way to declare "function returning
int taking no arguments" in pre-standard C, and K&R2 was written pretty
much in parallel with the first ANSI C standard.)

Any worthwhile book written much more recently than that should be written
by people who know that it has since become established as bad style
(but probably something people who have to deal with already-existing
code should know about anyways).


dave

 

[interpim]

IN MAIN()

#include "primes.h"

int main()
{
int count,x,y,n;

printf("PRIMES WILL BE PRINTED\n");
printf("How many do you want to see? ");
scanf("%d",&x);
for(count=0, n=2; count<x; ++n)
{
if (is_prime(n)==1)
{
printf("%5d: ",count+1);
printf("%5d\n",n);
++count;
}
}
}



IN PRIMES.H

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

int is_prime(int n);


IN IS_PRIME.C

#include "primes.h"
int is_prime(int n)
{
int k, limit;

if (n==2)
return 1;
if (n%2==0)
return 0;
limit =n/2;
for (k=3;k<=limit; k += 2)
if (n%k==0)
return 0;
return 1;
}

Thanks all again for your help.

 

[Kevin Goodsell]

IN MAIN()

#include "primes.h"

int main()

It's better to include 'void' inside the parens. '()' and '(void)' are
very different. In the case of main() it's arguable whether it matters,
but in general you should not declare functions with empty parameter
lists unless you *really* know what you are doing.

{
int count,x,y,n;

printf("PRIMES WILL BE PRINTED\n");
printf("How many do you want to see? ");

This prompt still might never be seen. I/O is often line-buffered. The
I/O system may wait until a complete line is available before flushing
the buffer to the output device. Add this:

fflush(stdout);

scanf("%d",&x);
for(count=0, n=2; count<x; ++n)
{
if (is_prime(n)==1)

Probably better to just say

if (is_prime(n))

It reads easier, and evaluates correctly for all "true" values that
is_prime() might return.

{
printf("%5d: ",count+1);
printf("%5d\n",n);

It's perfectly safe to combine these:

printf("%5d: %5d\n", count+1, n);

It's even a bit easier to understand, I think.

++count;

You could move this above the printf(), and lose the '+1'. You could
even combine all three of these statements this way:

printf("%5d: %5d\n", ++count, n);

But if you are more comfortable with this, I would understand:

++count;
printf("%5d: %5d\n", count, n);

}
}
}



IN PRIMES.H

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

I still think it's a bad idea to #include headers where they aren't
needed, particularly in a header file (because it means you are forcing
these to be #included in any code that uses your header, even if they
aren't wanted or needed).

<snip>

-Kevin

 

[Arthur J. O'Dwyer]

It's better to include 'void' inside the parens. '()' and '(void)' are
very different. In the case of main() it's arguable whether it matters,
but in general you should not declare functions with empty parameter
lists unless you *really* know what you are doing.

<OT> Note that the exact opposite is common practice in C++, where
() and (void) *are* synonyms in this context. But C isn't C++.
Just thought I'd point that out in case there was any confusion. </OT>

if (is_prime(n))

You could move this above the printf(), and lose the '+1'. You could
even combine all three of these statements this way:

printf("%5d: %5d\n", ++count, n);

But if you are more comfortable with this, I would understand:

++count;
printf("%5d: %5d\n", count, n);

IMHO the latter is always better. I don't generally expect
my 'printf' calls to have side effects. Also, the former way
might bite you if you ever want to, say, print a newline after
every few numbers:

++count;
printf("%5d: %5d%s", count, n, ((count%10)? " ": "\n"));

versus

/* WRONG WRONG WRONG WRONG WRONG */
printf("%5d: %5d%s", ++count, n, ((count%10)? " ": "\n"));

I still think it's a bad idea to #include headers where they aren't
needed, particularly in a header file (because it means you are forcing
these to be #included in any code that uses your header, even if they
aren't wanted or needed).

Right. Lose the headers. And add the missing include guards,
like so:

#ifndef H_PRIMES
#define H_PRIMES
...
#endif

<snip>

HTH,
-Arthur