1
\$\begingroup\$

Below is a C code that generates 1000 random positive integers, for each number we check primality, palindromic property, and the square root of the sum of the digits. The purpose is to find/discover positive integers with interesting/beautiful property such as:

"12345678910987654321 is a prime number"

or

"12321 is a palindromic number and the sum of its digits is a square number." (although this may not be that interesting)

Is there a better and efficient way to write the code? especially if I want it to include large integers.

Code:


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


int is_prime(unsigned long int n){
    
    unsigned int p;
    
    if (n == 2){
    return 1;
    }
    
    if (!(n & 1) || n < 2 ) {
            return 0;
    }
    
    for (p = 3; p <= n/p; p += 2){
    
    if (!(n % p)){
        return 0;}
    }
    
    return 1;

}

int palindrome(unsigned long int n){
    
    char strnum[20];
    int i, nstr;
    float cond;
    sprintf(strnum, "%ld", n);
    nstr = strlen(strnum);
    cond = (nstr-1)/2.0;
    
        for (i=0; i< cond; i++){
            if (strnum[i] != strnum[nstr-1-i]){
                return 0;
            }
        }
        return 1;
}
    
    
int sum_of_digits(unsigned long int n){
    
    char strnum[20];
    int i, nstr, term, total = 0;
    
    sprintf(strnum, "%ld", n);
    nstr = strlen(strnum);
    
    for (i=0; i<nstr; i++){
    
    term = ((int)(strnum[i])) - 48;
    total = total + term;
    
    }
    
    return total;
}
    
int main(){
    
    unsigned long int i;
    float added_digits;
    unsigned int prime_status, palindrome_status;
    int N = 1000;
    
    srand(4);
    for (int j=0; j < N; j++){
    
    i = rand();
    printf("Number = %ld\n", i);
    
    prime_status = is_prime(i);
    switch (prime_status){
    case 1:
        printf("Prime number status: True\n");
        break;
    default:
        printf("Prime number status: False\n");
    }
    
    palindrome_status = palindrome(i);
    
    if (palindrome_status==1){
        printf("The number is a Palindrome\n");
    }
    else{
        printf("The number is not a Palindrome\n");
    }
    
    added_digits = sqrt(sum_of_digits(i));
    printf("(Square root) sum of digits: %f\n\n", added_digits);
    
    }
}

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Your code indentation is all over the place. It's probably worth a pass through indent if your editor can't align the code for you. That certainly helps readers understand the structure.

We no longer like in the C90 world where all variables needed to be declared at the start of their scope, before any statements. It's better to declare variables when their initialisation is available.

is_prime()

Be careful using names that begin with is - we're okay here because the next character isn't a letter, but many C programmers don't properly know the rules of reserved identifiers, and may mistakenly change that to isprime (which wouldn't be allowed).

Let's include <stdbool.h>; then we can use the bool type for return values from is_prime() and palindrome().

We have no guarantee that unsigned int is large enough for p - it's possible for ULONG_MAX to be greater than UINT_MAX * UINT_MAX.

#include <stdbool.h>

bool is_prime(unsigned long n)
{
    if (n == 2) {
        return true;
    }

    if (n < 2 || n % 2 == 0) {
        return false;
    }

    /* simple primality test - could be improved */
    for (unsigned long p = 3;  p <= n / p;  p += 2) {
        if (n % p == 0) {
            return false;
        }
    }

    return true;
}

palindrome()

The constant 20 seems to be a guess at the longest unsigned long we could print. Unfortunately, C doesn't provide a constant to use here, but we should at least check for overrun - use snprintf() instead of sprintf() for that.

strlen returns an unsigned size_t, so avoid converting to signed int.

We don't need floating-point division, as in any odd-length string, the middle character is always equal to itself, so we can safely use integer division, letting it round down.

bool is_palindrome(unsigned long int n)
{
    char strnum[20];
    size_t nstr = snprintf(strnum, sizeof strnum, "%ld", n);
    if (nstr >= sizeof strnum) {
        fprintf(stderr, "is_palindrome: buffer size %zd too small for number %lu\n", sizeof strnum, n);
        return false;           /* we're not sure, so play safe */
    }

    const size_t midpoint = nstr / 2;
    for (size_t i = 0;  i < midpoint;  ++i) {
        if (strnum[i] != strnum[nstr-1-i]) {
            return false;
        }
    }
    return true;
}

sum_of_digits

Again, we should be using snprintf() to avoid buffer overrun. But instead of printing to a buffer, we can just use successive division by 10 to read the digits.

The magic number 48 seems to be a substitute for the character '0'. Use the character, which will work on more systems than just your own (C compiles for environments with many character codings). It also makes the intent much clearer to the reader.

unsigned sum_of_digits(unsigned long int n)
{
    unsigned total = 0;
    for (;  n;  n /= 10) {
        total += n % 10;
    }

    return total;
}

main()

rand() returns an int between zero and RAND_MAX, so using it to generate an unsigned long won't exercise the full range of values.

We should use %u conversion to print unsigned values.

Use if/else rather than switch for boolean values. We can simplify, anyway:

    printf("Prime number status: %s\n",
           is_prime(i) ? "True" : "False");

We can combine all the consecutive printf() into a single call.

sqrt() returns a double, so just use that instead of converting down to float.

int main(void)
{
    srand(4);

    static const int number_of_runs = 1000;
    for (int j=0;  j < number_of_runs;  ++j) {
        unsigned long int i = rand();
        printf("Number = %lu\n"
               "Prime number status: %s\n"
               "The number %s a Palindrome\n"
               "Square root of digital sum: %f\n\n",
               i,
               is_prime(i) ? "True" : "False",
               is_palindrome(i) ? "is" : "is not",
               sqrt(sum_of_digits(i)));
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ why number_of_runs must be static? \$\endgroup\$
    – Redsbefall
    Commented Feb 24, 2022 at 13:13
  • \$\begingroup\$ It's more that because it's a constant, it has no need to be non-static. In main(), the distinction probably isn't important, as there will only ever be one invocation. \$\endgroup\$ Commented Feb 24, 2022 at 13:21
  • \$\begingroup\$ since it is not used in another function, then no need to be static right? \$\endgroup\$
    – Redsbefall
    Commented Feb 24, 2022 at 13:24
  • \$\begingroup\$ As I said, it doesn't need to be static. But it's a good practice for your constants. \$\endgroup\$ Commented Feb 24, 2022 at 15:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.