using a function to calculate water bill in c

Question

I'm trying to write a program that uses a function to calculate a water bill. It reads the information about water usage from a file, then calculates the water bill after tax.

This is the file:

g 5000
B 1250
M 50

This is what the output should be:

Type  Water usage  Cost including tax($)
g     5000              194.30
B        1250              93.89
Wrong user type

This is my output:

Type  Water usage  Cost including taxWrong user type.
g     5000          0.000000
B     1250          18.750750
Wrong user type.

I'm not sure if the problem lies in my formula or something else. There's obviously a problem with the if else statement in the function since it keeps printing "Wrong user type" where it shouldn't.

This is my code:

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <math.h>

double water_billcalculation(char user_type, double water_use, double bft, double at);

int main(void) {
    FILE* water;
    water = fopen("water_usage.txt", "r");

    char user_type;
    int cf;
    double bft = 0, at = 0, water_use = 0;

    printf("Type  Water usage  Cost including tax");

    while (fscanf(water, "%c%d ", &user_type, &cf) != EOF) {
        //water_billcalculation(user_type, water_use, bft, at);
        printf("%c     %d          %lf\n", user_type, cf, water_billcalculation(user_type, water_use, bft, at));
    }

    return(0);
}

double water_billcalculation(char user_type, double water_use, double bft, double at) {
    if (user_type == 'G') { 
        bft = (water_use * 0.035) + 3.75;
        at = bft + (bft * .087);
    }
    else if (user_type == 'B') { 
        bft = (water_use * .0553) + 17.25;
        at = bft + (bft * .087);
    }
    else if (user_type == 'R') { 
        if (water_use <= 400) {
            bft = (water_use * .04) + 13.5;
            at = bft + (bft * .087);
        }
        else if (water_use > 400 && water_use <= 700) {
            bft = (water_use * .062) + 13.5;
            at = bft + (bft * .087);
            
        }
        else {
            bft = (water_use * .12) + 13.5;
            at = bft + (bft * .087);
        }
    }
    else { 
        printf("Wrong user type.\n");
    }
    return(at);
}

Answer 1

The problem

You're "reading" your file in a wrong way. Change this line:

    while (fscanf(water, "%c%d ", &user_type, &cf) != EOF) {

to this (notice the difference in the second argument of fsanf):

    while (fscanf(water, " %c%d", &user_type, &cf) == 2) {

Also your water_billcalculation is wrong: You're looking after the user_type G, B and R according to your code, but you're actually looking after g, B and M! So you'd need to change this part:

    if (user_type == 'G') { 
        bft = (water_use * 0.035) + 3.75;
        at = bft + (bft * .087);
    }
    else if (user_type == 'B') { 
        bft = (water_use * .0553) + 17.25;
        at = bft + (bft * .087);
    }
    else if (user_type == 'R') { 

to this:

    if (user_type == 'g') { 
        bft = (water_use * 0.035) + 3.75;
        at = bft + (bft * .087);
    }
    else if (user_type == 'B') { 
        bft = (water_use * .0553) + 17.25;
        at = bft + (bft * .087);
    } else if (user_type == 'M') { 

After this, you'll get the following output:

Type  Water usage  Cost including taxg     5000          4.076250
B     1250          18.750750
M     50          14.674500

I'm a little bit unsure, if this is your desired output, because according to your post which says, that you'd like to have this output:

Type  Water usage  Cost including tax($)

g     5000              194.30

B        1250              93.89
 
Wrong user type

makes me thinking that:

• You actually don't want to "parse" the type M. If yes, remove the else if (user_type == 'M') condition in the water_billcalculation function.
• The calculation isn't correct or I don't see another bug in my code (althought I even ran it with valgrind, which didn't complain about any invalid actions).

Code review

Some improvement ideas came up in my mind, when I read your code. So here are some suggestions. You can skip this, if you aren't interested in them.

Check the return value after calling fopen!

fopen returns you a FILE * but only IF everything worked fine! So please make sure to add a lookup part:

    FILE * water;
    water = fopen("water_usage.txt", "r");

    if (water == NULL) {
        // Thanks to @William Pursell for pointing out to use stderr for error messages, I forget that pretty often
        perror("Houston, we've got a problem: The file couldn't be opened :(\n");
        // or use the `exit()` function here (but you'd need to include stdlib.h)
        return 1;
    }

String formatting

I'd recommend to use \t instead of counting your spaces. This would let you produce a better output. Also don't forget to add a \n if you're printing something in a new context.

In the beginning when I've got this ouptut:

Type  Water usage  Cost including taxg     5000          4.076250
B     1250          18.750750
M     50          14.674500

I was a little bit irritated first, because I didn't saw the line with the g type.

Change your output to the following:

    printf("Type\tWater usage\tCost including tax\n");

    while (fscanf(water, "%c %d\n\n", &user_type, &cf) != EOF) {
        printf("%c\t%d\t\t%lf\n", user_type, cf, water_billcalculation(user_type, water_use, bft, at));
    }

This gives me the following output:

Type    Water usage     Cost including tax
g       5000            4.076250
B       1250            18.750750
M       50              0.000000

which is better to read (in my opinion).

(Optional) use switch-case

Your water_billicalculation could include a switch-case statement instead of nested if-else statements.

I'd have the written the function as follows:

double water_billcalculation(char user_type, double water_use, double bft, double at) {

    switch (user_type) {
        case 'g':
            bft = (water_use * 0.035) + 3.75;
            at = bft + (bft * .087);
            break;
        case 'B':
            bft = (water_use * .0553) + 17.25;
            at = bft + (bft * .087);
            break;
        case 'M':
            if (water_use <= 400) {
                bft = (water_use * .04) + 13.5;
                at = bft + (bft * .087);
            }
            else if (water_use > 400 && water_use <= 700) {
                bft = (water_use * .062) + 13.5;
                at = bft + (bft * .087);
                
            }
            else {
                bft = (water_use * .12) + 13.5;
                at = bft + (bft * .087);
            }
            break;
        defaut:
            fprintf(stderr, "Wrong user type: '%c'\n", user_type);
            break;
    }
    return at;
}

instead of this:

    if (user_type == 'g') { 
        bft = (water_use * 0.035) + 3.75;
        at = bft + (bft * .087);
    }
    else if (user_type == 'B') { 
        bft = (water_use * .0553) + 17.25;
        at = bft + (bft * .087);
    }
    else if (user_type == 'M') { 
       // and so on...

but that's probably very individual, so see this as an opinion please.

Answer 2

While @TornaxO7 answered your literal question, you would have been able to easily determine what the problem is by debugging your program:

How to debug a C program

Specifically,

• By stepping your program or breaking after the scan, you would have noticed what values you're getting in user_type and in cf.
• By stepping through the comparisons in water_billcalculation(), you would have noticed how comparisons you expected to succeed, fail - which would have led you to notice you're comparing 'g' with 'G'.

Additionally, or alternatively, adding some "debug-prints" or log-type printing to your program, at least while developing it, provides some of the same information even in regular runs without debugging.