Parking Charges exercise from Deitel's C book

Question

I'm learning to code with Deitel's C: How to program book, 6th edition. Here's Exercise 5.9 (Parking Charges):

A parking garage charges a $2.00 minimum fee to park for up to three hours and an additional $0.50 per hour for each hour or part thereof over three hours. The maximum charge for any given 24-hour period is $10.00. Assume that no car parks for longer than 24 hours at a time. Write a program that will calculate and print the parking charges for each of three customers who parked their cars in this garage yesterday. You should enter the hours parked for each customer. Your program should print the results in a neat tabular format, and should calculate and print the total of yesterday's receipts. The program should use the function calculateCharges to determine the charge for each customer.

My solution is below. These exercises have no answers so I don't know how can I improve this code, in which I want to know if it is possible to dispense with variables "one", "two" and "three", and also automate the printing process (after the for loop in main) inside a loop.

#include <stdio.h>

float calculateCharges( float hours );

float charge;

int main()
{
    int customer;
    float one;
    float two;
    float three = 0;
    float hours;

    for( customer = 1; customer <= 3; customer++ ) {
        printf( "Enter customer #%d parking hours: ", customer );
        scanf( "%f", &hours );

        if( customer == 1 )
            one = hours;
        else if( customer == 2 )
            two = hours;
        else
            three = hours;
    }
    printf( "%s%10s%12s", "Car", "Hours", "Cost" );
    printf( "\n%d%12.1f%12.1f", 1, one, calculateCharges( one ) );
    printf( "\n%d%12.1f%12.1f", 2, two, calculateCharges( two ) );
    printf( "\n%d%12.1f%12.1f", 3, three, calculateCharges( three ) );
    printf( "\n%s%8.1f%12.1f", "TOTAL", one + two + three, calculateCharges( one ) + calculateCharges( two ) + calculateCharges( three ) );
}

float calculateCharges( float hours )
{
    int h = hours;
    charge = 2.0;

    if( hours > 0 ) {
        if( hours <= 3 )
            return charge;

        else if( hours <= 24 ) {
            while( h > 3 ) {
                charge += .5;
                h--;
                if( charge >= 10 )
                    charge = 10;
            }
            return charge;
        }
    }
    else
        printf( "\nThe amount of time entered is not supported." );
}

Answer 1

I see a number of things that may help you improve your code.

Eliminate global variables where practical

The code declares and uses a global variable charge. Global variables obfuscate the actual dependencies within code and make maintainance and understanding of the code that much more difficult. It also makes the code harder to reuse. For all of these reasons, it's generally far preferable to eliminate global variables by either making them local and/or passing them into and out of functions. That way the linkage is explicit and may be altered more easily if needed. For example, to eliminate charge global, simply make it local to calculateCharges.

Consider improving names

I think hours is not a bad variable name, but one is, since hours gives the reader some idea of why the variable exists, but one could be anything.

Check return values for errors

The call to scanf can fail. You can check the return values to make sure they haven't or your program may crash (or worse) when given malformed input or due to low system resources. Rigorous error handling is the difference between mostly working versus bug-free software. You should strive for the latter.

Understand variable passing

When you write this:

float calculateCharges( float hours )

It means that the function takes a float named hours as the argument. Because it doesn't say float &hours, which would take a reference to the passed variable (a pointer, in essence), it's passed by by value. This means that it's really not necessary to create a local copy named h; just use hours directly.

Decompose the program into smaller parts

Right now, much of the code is in main which isn't necessarily wrong, but it means that it's not only hard to reuse but also hard to troubleshoot. Better is to separate the code into small chunks. It makes it both easier to understand and easier to fix or improve. For example, one might have a function for getting input and another for printing output in addition to the existing function that converts hours to a charge.

Make sure all paths return a value

The calculateCharges routine returns the dollar amount if the passed number of hours is greater than zero, but what if it's not? Right now the code doesn't return any value which is a problem. Rather than printing a message, better would be to return some special marker value, such as -1 that the calling code can check for and then have it decide the appropriate action.

Use an array

Instead of naming things one, two, three, create an array: float hours[3]; and loop through that.