Calculate the sum of interior angles of a polygon - follow-up

Question

This is a follow-up review of Calculate the sum of interior angles of a polygon.

Using the helpful answers from the original post, I have integrated the suggestions which are in order from first to last:

• Adding checks to scanf().
• Using main(void) in place of main(...).
• Passing char * directly to printf().
• Made conditionals simpler.
• main() is broken up into several functions and subroutines.

I am seeking a second peer review because I have also added changes not suggested in the original post which are:

• Using enumerations to convey error information.
• Heavier error-checking in function askAgain().

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


/* Keep string constants of the questions. */

const char* howManySides =
"How many sides does the shape have? ";

const char* askAnotherShape =
"Would you like to find interior angles for another polygon? (0 or 1) ";

/* Table of polygon names from 3 to 12 sides */
const char* names[] = {
    "a triangle",
    "a quadrilateral",
    "a pentagon",
    "a hexagon",
    "a heptagon",
    "an octagon",
    "a nonagon",
    "a decagon",
    "a hendecagon",
    "a dodecagon"
};

enum errorType {noError, errNegSides, errZeroSides, errInsufSides};



static int getSumInteriorAngles(const unsigned int numSides)
{
    return 180 * (numSides - 2);
}



/* Print a specific type of error based on its enumeration.
   */
static void printError(const enum errorType eType)
{
    switch(eType) {
        case errNegSides:
        printf("The number of sides cannot be negative...\n");
        break;

        case errZeroSides:
        printf("The number of sides cannot be zero...\n");
        break;

        case errInsufSides:
        printf("The shape must have at least 3 sides...\n");
        break;
    }
}



/* Takes a number of polygon sides and returns a type of error if the number
   of sides invalid or no error type if the number of sides is valid.
   */
static enum errorType isSidesValid(const int numSides)
{
    if(numSides < 0) return errNegSides;
    else if(numSides == 0) return errZeroSides;
    else if(numSides < 3) return errInsufSides;
    else return noError;
}



static int getUserInput(void)
{
    int userInput;
    bool isValidResponse = false;

    while(!isValidResponse) {
        /* Ask for the number of sides. */
        printf(howManySides);

        /* We're looking for exactly 1 argument, so if there's
           less than that, then something went wrong with the conversion. */
        if(scanf("%i", &userInput) < 1) {
            printf("Not a number...\n");
            fflush(stdin);

            /* This check failed, so repeat the question. */
            continue;
        }
        isValidResponse = true;
    }
    return userInput;
}



static int getNumSides(void)
{
    int numSides;

    /* Initialize the checks flag to false and make it true only when
       all checks pass. */
    bool passedAllChecks = false;


    while(!passedAllChecks) {
        numSides = getUserInput();

        /* If the input was a valid number, then we have to check if the
           number of sides makes sense. */
        enum errorType returnError = isSidesValid(numSides);

        if(returnError != noError) {
            /* If the number doesn't make sense for a number of sides,
               then print the specific error and repeat the question. */
            printError(returnError);
            continue;
        }

        /* All checks passed. Exit the loop and return the number of sides. */
        passedAllChecks = true;
    }
    return numSides;
}



static int askAgain(void)
{
    int userResponse = 0;
    bool isValidResponse = false;

    while(!isValidResponse) {
        printf(askAnotherShape);

        /* Weed out non-integers and other weirdness. */
        if(scanf("%i", &userResponse) < 1) {
            printf("Not a valid response...\n");
            fflush(stdin);
            continue;
        }

        /* Restrict valid input to 0 or 1. */
        if((userResponse != 0) && (userResponse != 1)) {
            printf("Please choose 0 or 1...\n");
            continue;
        }
        isValidResponse = true;
    }
    return userResponse;
}



static void printResult(int numSides)
{
    /* Assume we're dealing with a polygon without a special name,
       in which case, we'll refer to it non-specifically. */
    const char *name = "this polygon";

    /* If we have a name for this polygon, then get it from the table. */
    if(numSides <= 12) {
        name = names[numSides - 3];
    }

    printf("The sum of the interior angles of %s is %i\n",
            name, getSumInteriorAngles(numSides));
}



int main(void)
{
    do {
        printResult(getNumSides());
    } while(askAgain());

    return 0;
}

```

Answer 1

String as formats

Code uses printf(howManySides); yet howManySides is not described as a format. Such practice can lead to trouble when the string contains a %. Do not use a string as a format unless it is clearly stated in its definition that it is a format.

// printf(howManySides);
fputs(howManySides, stdout);

Ensure output before input

stdout may be byte, line, or fully buffered. Insure output is seen before requesting user input. Often printing a '\n' is sufficient - except that the code here does not have that in the prompts. fflush(stdout) always works.

printf(howManySides);
fflush(stdout); // add
if(scanf("%i", &userInput) < 1) {

Avoid undefined behavior

See why not to use fflush(stdin).

Tip: ditch scanf(); use fgets() for robust error handing.

Infinite loop

Should scanf("%i", &userInput) < 1) return EOF due to end-of-file, code loops infinitely. Instead, when the return is EOF, consider ending the program or at least get out of the loop.

Avoid magic numbers

Rather than hard code 3, 12 and their effects in various places in code, create constants.

#define POLY_MIN 3
#define POLY_N   (sizeof names/sizeof names[0])
#define POLY_MAX (POLY_N + POLY_MIN - 1)

// if(numSides <= 12) {
//  name = names[numSides - 3];
if(numSides <= POLY_MAX) {
  name = names[numSides - POLY_MIN];

Avoid naked equations

Sure 180 * (numSides - 2) is grade school math, yet often a reference is useful.

// en.wikipedia.org/wiki/Internal_and_external_angles#Properties
return 180 * (numSides - 2);

---

Minor stuff

Curious type selection

Unclear why getSumInteriorAngles() uses unsigned. Function is only used once and it is called with int

// static int getSumInteriorAngles(const unsigned int numSides)
static int getSumInteriorAngles(const int numSides)

Sometimes const, sometimes not

Some functions declare parameters as const and others do not, even though the parameter is not modified in both. Hmmm. IAC, I see it more as clutter than useful. As with such style issues, code to your group's coding standard.

do loop candidate

bool isValidResponse = false;
while(!isValidResponse) {
  ...
}

Answer 2

We can eliminate the magic number 12 here:

/* If we have a name for this polygon, then get it from the table. */
if(numSides <= 12) {
    name = names[numSides - 3];
}

The common idiom for determining the number of elements in an array works by using the size of the array and the size of each element:

static const int max_sides = sizeof names / sizeof *names;

We can eliminate the constant 3 even more easily, by including dummy (unused) elements at the beginning of the array:

/* polygon names up to 12 sides */
const char* names[] = {
    "(point)",
    "(line)",
    "(line)",
    "a triangle",
    "a quadrilateral",
    "a pentagon",
    "a hexagon",

Here, I've included some descriptive strings for the invalid cases, which makes debugging easier should we accidentally use one of them. We could save a tiny bit of program size by making them null pointers instead.

With those changes, the code I quoted becomes this:

/* If we have a name for this polygon, then get it from the table. */

static const int max_sides = sizeof names / sizeof *names;

if (numSides < max_sides) {
    name = names[numSides];
}

Or, making it constant:

const char *const name = numSides < max_sides
                         ? names[numSides]
                         : "this polygon";