Goto and Code Repetition - Are they avoidable in this case?

Question

I've recently faced a programming problem, and it seems to me that the most optimized way of solving it is by using goto, even though it's not a good practice. The problem is: tell the user to enter a positive natural number ( > 0) and read the input. If this number is valid, tell the user the square of that number. Do this while the input is correct. I came up with a few solutions, but all of them seem to have problems. Here are two of them:
Solution 1 - Problem: Uses goto

#include <stdio.h>

int main()
{
    int num;

_LOOP:
    printf("Enter a positive natural number: ");
    scanf("%i", &num);

    if (num > 0) {
        printf("Square: %i\n", num * num);
        goto _LOOP;
    }

    printf("Invalid number\n");

    return 0;
}

Solution 2 - Problem: Double-checks if num > 0 (code repetition)

#include <stdio.h>

int main()
{
    int num;

    do {
        printf("Enter a positive natural number: ");
        scanf("%i", &num);

        if (num > 0)
            printf("Square: %i\n", num * num);
    } while (num > 0);
    
    printf("Invalid number\n");

    return 0;
}

Obviously, there are more ways to solve the problem, but all the other ones I came up with that do not use goto encouter the same code repetition problem. So, is there a solution where both goto and code repetitions are avoided? If not, which one should I go for?

Answer 1

I am a bit surprised nobody suggested functional decomposition yet. Instead of writing a big pile of primitive statements, cut main up into smaller functions. Apart from readability/maintainability benefits, it also helps eliminate code duplication in a very natural way.

In OP's case, getting input from the end user is a separate responsibility, and makes a good choice for a separate function.

static bool user_enters_number(int *ptr_to_num)
{
    printf("Enter a positive natural number: ");
    return scanf("%i", ptr_to_num) == 1;
}

Notice user_enters_number explicitly tests the return value of scanf. This improve end-of-file handling.

Likewise, you could give number validation its own function. This may seem like overkill (it's just num > 0, right?), but it gives us the opportunity to combine the validation with the resulting error message. Printing "Invalid number" at the end of main feels wrong. Invalid numbers are not the only exit condition; end-of-file is another. So instead, I will let the validation function determine the message. As a bonus, this makes it possible to support multiple error types (e.g. separate messages for negative numbers and for zero).

static bool is_valid_number(int num)
{
    bool ok = (num > 0);
    if (!ok) printf("Invalid number\n");
    return ok;
}

We now have two boolean-typed functions that can be neatly chained together with && and put inside the condition part of a loop, which is an idiomatic way of saying: if either one of these functions fails (i.e. returns false), immediately exit the loop.

What's left, is a very clean main function.

int main(void)
{
    int num;
    while (user_enters_number(&num) && is_valid_number(num))
    {
        printf("Square: %i\n", num * num);
    }
}

To appreciate the benefits in terms of maintainability, try rewriting this code so that it accepts two numbers and prints their product.

int main(void)
{
    int num1, num2;
    while (user_enters_number(&num1) && is_valid_number(num1) &&
           user_enters_number(&num2) && is_valid_number(num2))
    {
        printf("Product: %i\n", num1 * num2);
    }
}

The changes are trivial, and limited to a single function (though you might consider adding a parameter input_prompt to user_enters_number).

There is no performance penalty for this 'divide-and-conquer' approach: a smart compiler will do whatever is necessary to optimize the code, e.g. inline the functions.

Answer 2

For starters if you expect a non-negative number then the variable num should have an unsigned integer type for example unsigned int.

As it is written in your question the user can enter an invalid data or interrupt the input. You have to process such a situation.

Also the multiplication num * num can result in an overflow.

And using goto instead of a loop is indeed a bad idea.

Pay attention to that you should declare variables in minimum scopes where they are used.

To use the for loop for such a task is also a bad idea. It is much expressive to use a while loop.

The program can look the following way

#include <stdio.h>
#include <stdbool.h>

int main(void) 
{
    while ( true )
    {
        printf( "Enter a positive natural number: " );
        
        unsigned int num;

        if ( scanf( "%u", &num ) != 1 || num == 0 ) break;

        printf( "Square: %llu\n", ( unsigned long long )num * num );
    }

    puts( "Invalid number" );

    return 0;
}

The program output might look like

Enter a positive natural number: 100000000
Square: 10000000000000000
Enter a positive natural number: 0
Invalid number

Or it would be even better to move the last output statement inside the while statement. Fo rexample

#include <stdio.h>
#include <stdbool.h>

int main(void) 
{
    while ( true )
    {
        printf( "Enter a positive natural number: " );
        
        unsigned int num;

        if ( scanf( "%u", &num ) != 1 || num == 0 )
        {
            puts( "Invalid number" );
            break;
        }
        
        printf( "Square: %llu\n", ( unsigned long long )num * num );
    }

    return 0;
}

Answer 3

The other option: check a bool that stores if the continue condition is met. It reads much easier than the infinite loop/break approaches (to me) and there's no code repetition.

#include <stdio.h>
#include <stdbool.h>

int main()
{
    int num;
    bool bContinue;

    do {
        printf("Enter a positive natural number: ");
        scanf("%i", &num);

        if (num > 0){
            printf("Square: %i\n", num * num);
            bContinue = true;
        }
        else{
            printf("Invalid number\n");
            bContinue = false;
        }
    } while (bContinue);             

    return 0;
}

Answer 4

Here's half the answer; try to fill in what's missing. Remember that sometimes it's better to structure your loop as "do something until..." rather than "do something while..."

    for (;;) {
        printf("Enter a positive natural number: ");
        scanf("%i", &num);
        if (num <= 0)
            break;
        printf("Square: %i\n", num * num);
    }
    printf("Invalid number\n");

[updated with @rdbo's answer]

Answer 5

What about breaking out of the loop? This is basically a goto statement to the end of the loop without explicitly using goto.

#include <stdio.h>

int main()
{
    int num;

    while(1) {
        printf("Enter a positive natural number: ");
        scanf("%i", &num);

        if (num > 0) {
            printf("Square: %i\n", num * num);
        } else {
            printf("Invalid number\n");
            break;
        }
    }

    return 0;
}