patterncMajor
First prime number larger than given integer
Viewed 0 times
numberlargerthanfirstprimegiveninteger
Problem
How can I test this C program for "efficiency"? The most interesting usage is that it returns negative output for large enough input, otherwise the behavior is about expected. Will you suggest how to improve the code, both the ui used in main that should be both efficient and preferably platform-indepedent and not use too much libraries for portability and optimization. I think the obvious improvement that I can make is recoding the
```
/*
* NextPrime
*
* Return the first prime number larger than the integer
* given as a parameter. The integer must be positive.
*/
#define PRIME_FALSE 0 / Constant to help readability. /
#define PRIME_TRUE 1 / Constant to help readability. /
#include
#include
int nextprime(int inval) {
int perhapsprime; / Holds a tentative prime while we check it. /
int testfactor; / Holds various factors for which we test perhapsprime. /
int found; / Flag, false until we find a prime. /
if (inval > 1) + 1; testfactor
+= 1) {
found = PRIME_TRUE; / Assume we will find a prime. /
if ((perhapsprime % testfactor) == 0) / If testfactor divides perhapsprime... /
{
found = PRIME_FALSE; / ...then, perhapsprime was non-prime. /
goto check_next_prime;
/ Break the inner loop, go test a new perhapsprime. /
}
}
check_next_prime: ; / This label is used to break the inner loop. /
if (found == PRIME_TRUE) / If the loop ended normally, we found a prime. /
{
return (perhapsprime); / Return the prime we found. /
}
}
return (perhapsprime); / When the loop ends, perhapsprime is a real prime. /
}
int main(int argc, char *argv[]) {
int intvar;
if (sscanf(argv[1], "%i", &intvar) != 1) {
printf("please use an integer parameter to compute the next prime\n");
} else
printf("%d => %d\n", int
int to unsigned int: ```
/*
* NextPrime
*
* Return the first prime number larger than the integer
* given as a parameter. The integer must be positive.
*/
#define PRIME_FALSE 0 / Constant to help readability. /
#define PRIME_TRUE 1 / Constant to help readability. /
#include
#include
int nextprime(int inval) {
int perhapsprime; / Holds a tentative prime while we check it. /
int testfactor; / Holds various factors for which we test perhapsprime. /
int found; / Flag, false until we find a prime. /
if (inval > 1) + 1; testfactor
+= 1) {
found = PRIME_TRUE; / Assume we will find a prime. /
if ((perhapsprime % testfactor) == 0) / If testfactor divides perhapsprime... /
{
found = PRIME_FALSE; / ...then, perhapsprime was non-prime. /
goto check_next_prime;
/ Break the inner loop, go test a new perhapsprime. /
}
}
check_next_prime: ; / This label is used to break the inner loop. /
if (found == PRIME_TRUE) / If the loop ended normally, we found a prime. /
{
return (perhapsprime); / Return the prime we found. /
}
}
return (perhapsprime); / When the loop ends, perhapsprime is a real prime. /
}
int main(int argc, char *argv[]) {
int intvar;
if (sscanf(argv[1], "%i", &intvar) != 1) {
printf("please use an integer parameter to compute the next prime\n");
} else
printf("%d => %d\n", int
Solution
Things you could improve
Efficiency
-
As @rolfl stated, you want to use a Sieve of Eratosthenes (+1 to that answer).
-
I can't see if you are doing this already, but you should be compiling with your compiler's highest optimization level. With GCC for example, this would be
Portability
-
Keep in mind to adhere to the standards as closely as possible, because a system that supports C must adhere to the standards as well.
-
If you want your program to be as portable as possible, take a look into the AutoTools suite for packaging your program. This is a bit extreme in your case, since you aren't using external libraries, and your program doesn't have a lot of system dependencies.
Standards
-
You don't have to return
C99 & C11 §5.1.2.2(3)
...reaching the
value of
-
Use the standard library `
-
You can reduce down one of your test conditionals.
Efficiency
-
As @rolfl stated, you want to use a Sieve of Eratosthenes (+1 to that answer).
-
I can't see if you are doing this already, but you should be compiling with your compiler's highest optimization level. With GCC for example, this would be
-O3.Portability
-
Keep in mind to adhere to the standards as closely as possible, because a system that supports C must adhere to the standards as well.
-
If you want your program to be as portable as possible, take a look into the AutoTools suite for packaging your program. This is a bit extreme in your case, since you aren't using external libraries, and your program doesn't have a lot of system dependencies.
Standards
-
You don't have to return
0 at the end of main(), just like you wouldn't bother putting return; at the end of a void-returning function. The C standard knows how frequently this is used, and lets you not bother.C99 & C11 §5.1.2.2(3)
...reaching the
} that terminates the main() function returns avalue of
0.-
Use the standard library `
instead of defining your own boolean values.
#define PRIME_FALSE 0 /* Constant to help readability. */
#define PRIME_TRUE 1 /* Constant to help readability. */
It still helps with readability. And since it is part of the standard in C, systems are guaranteed to have it if they support C.
Syntax/Styling
-
Don't use goto.
Yes, there are some rare situations where you may find it necessary to use it. This is not one of them. A simple break; will suffice in it's place, since you are only breaking out of the inner loop.
-
Right now you are using a somewhat odd way to test if you have an integer input into the command line arguments.
if (sscanf(argv[1], "%i", &intvar) != 1)
Use argc to test the number of arguments input into the program instead, and isdigit to test if the input contains numbers.
if (argc != 2 || !isdigit(**(argv+1))) printf("Please use an integer parameter to compute the next prime.\n");
Then convert that input into an integer.
int intvar = atoi(*(argv+1));
Note that the order I put it in is important. We want to test argc first, so we put it first in the test conditional. If that fails, the second part on the other side of the || isn't even evaluated, and the statements following the conditional are executed. If we switched those tests around, undefined behavior could occur if we didn't enter in a command line argument.
-
Use puts() instead of printf() when you aren't formatting your strings. This allows you to not worry about the newline (\n) character at the end of your printed out strings.
puts("Please use an integer parameter to compute the next prime.");
-
You don't modify the value of intval within your nextprime()` function, therefore the parameter should be declared constant.int nextprime(const int inval)-
You can reduce down one of your test conditionals.
if (inval == 1)
return (2); /* Easy special case. */
if (inval == 2)
return (3); /* Easy special case. */if (inval == 1 || inval == 2) return (inval + 1); /* Easy special case. */Code Snippets
#define PRIME_FALSE 0 /* Constant to help readability. */
#define PRIME_TRUE 1 /* Constant to help readability. */if (sscanf(argv[1], "%i", &intvar) != 1)if (argc != 2 || !isdigit(**(argv+1))) printf("Please use an integer parameter to compute the next prime.\n");int intvar = atoi(*(argv+1));puts("Please use an integer parameter to compute the next prime.");Context
StackExchange Code Review Q#45744, answer score: 22
Revisions (0)
No revisions yet.