6
\$\begingroup\$

Of the languages that I use, I have the most experience with C++. I am looking to sharpen my skills with C, so I wrote this small calculator program.

What I have here is a sample of how I would probably write larger programs.

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



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



int main(int argc, char** argv)
{
    /* 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? (1 or 0) ";

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

    /* Input variable */
    int numSides = 0;

    /* Assume that the input is invalid. */
    int sidesValid = 0;

    /* Input variable */
    int askAgain = 0;

    do {
        do {
            sidesValid = 0;

            /* Ask for the number of sides. */
            printf(howManySides);

            /* Get the user input. */
            scanf("%i", &numSides);

            /* Handle invalid input. */
            if(numSides < 0) {
                printf("The number of sides cannot be negative...\n");
            }
            else if(numSides == 0) {
                printf("The number of sides cannot be zero...\n");
            }
            else if(numSides < 3) {
                printf("The shape must have at least 3 sides...\n");
            }
            else sidesValid = 1;
        }
        while(sidesValid == 0);

        /* If we have a name for this polygon, then get it from the table. */
        if(numSides <= 12) {
            char shapeName[128];
            /* Don't forget to copy the string terminator, so add 1 to the
               copy length. */

            memcpy(shapeName, names[numSides - 3],
                strlen(names[numSides - 3]) + 1);

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

        /* If we don't have a special name for this polygon, then just refer
           to it non-specifically. */

        else if(numSides > 12) {
            printf("The sum of the interior angles of this polygon is %i\n",
                getSumInteriorAngles(numSides));
        }

        printf(askAnotherShape);
        scanf("%i", &askAgain);

    } while(askAgain);

    return 0;
}
\$\endgroup\$

2 Answers 2

10
\$\begingroup\$

The most serious problem I see here is failing to check the return value from scanf(). Never ignore that - it's the only way we have to determine whether conversions were successful.

A less serious issue is that we we're using a main() that takes command-line arguments, but never use them. Prefer instead the no-argument version:

int main(void)

The copying of the shape's name isn't useful - we can just pass the pointer directly to printf():

        printf("The sum of the interior angles of a %s is %i\n",
               names[numSides - 3],
               getSumInteriorAngles(numSides));

We could even choose which to print by using a suitable pointer:

const char* names[] = {
    "a triangle",
    "a quadrilateral",
    "a pentagon",
    "a hexagon",
    "a heptagon",
    "an octagon",
    "a nonagon",
    "a decagon",
    "a hendecagon",
    "a dodecagon"
};

// ...

const char *name = "this polygon";
if (numSides <= 12) {
    name = names[numSides - 3];
}

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

This also stops the code printing the ungrammatical "a octagon". :-)

\$\endgroup\$
1
5
\$\begingroup\$

Complexity

Breaking problems into smaller and smaller parts until it is easy to solve is a standard part of software design and programming. Small functions make it easier to write, read, debug and maintain code. Most variables will be local variables and that cleans up the code as well.

The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

In addition to the function int getSumInteriorAngles(const unsigned int numSides) that already calculates the sum of the interior angles here are at least 3 possible functions in main().
- Get and validate the user input for the number of vertices
- Print the result
- Get and validate user input for if they want to go again.

In addition to the suggested functions above, some of the if statements are too complex. When showing the output the addition if in the following code isn't necessary

        else if(numSides > 12) {
            printf("The sum of the interior angles of this polygon is %i\n",
                getSumInteriorAngles(numSides));
        }

Only the else { is necessary.

Using some code from another answer, here is an alternate solution

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

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

static int getAndValidateNumberOfSides()
{
    int numSides = 0;
    bool sidesValid = false;

    do {
        sidesValid = 0;

        printf("How many sides does the shape have? ");
        scanf("%i", &numSides);

        if(numSides < 0) {
            printf("The number of sides cannot be negative...\n");
        }
        else if(numSides == 0) {
            printf("The number of sides cannot be zero...\n");
        }
        else if(numSides < 3) {
            printf("The shape must have at least 3 sides...\n");
        }
        else sidesValid = true;
    } while(!sidesValid);

    return numSides;
}

static bool askAgain()
{
    int goAgain = 0;

    printf("Would you like to find interior angles for another polygon? (1 or 0) ");
    scanf("%i", &goAgain);

    return goAgain == 1;
}

static void showResults(int numSides)
{
    const char* names[] = {
            "a triangle",
            "a quadrilateral",
            "a pentagon",
            "a hexagon",
            "a heptagon",
            "an octagon",
            "a nonagon",
            "a decagon",
            "a hendecagon",
            "a dodecagon"
    };

    int SumInteriorAngles = getSumInteriorAngles(numSides);

    if(numSides <= 12) {
        printf("The sum of the interior angles of %s is %d\n", names[numSides - 3], SumInteriorAngles);
    }
    else {
        printf("The sum of the interior angles of this polygon is %d\n", SumInteriorAngles);
    }
}

int main()
{
    do {
        int numSides = getAndValidateNumberOfSides();
        showResults(numSides);
    } while(askAgain());

    return 0;
}
\$\endgroup\$

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.