HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Summarizing calories for a food diary

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
summarizingdiaryforcaloriesfood

Problem

I'm new to C and I've been working on some code for a project for school. This was the finished result and now that I'm done it, I came back and tried making it better. So I was wondering, how would I make this printAvgCalPerGroup simplified?

```
#include "diary.h"

void printTotalCal(Food * list)
{
int total = 0;
Food * temp = list;

while (!isEmpty(temp))
{
total += (int)temp->calories;
temp = temp->next; // change to the next list item
}

printf("%d\n", total);
return;
}

void printAvgCalPerGroup(Food * list)
{
double vegCount = 0.0;
double meatCount = 0.0;
double dairyCount = 0.0;
double grainsCount = 0.0;
double fatCount = 0.0;

double vAmnt = 0.0;
double mAmnt = 0.0;
double dAmnt = 0.0;
double gAmnt = 0.0;
double fAmnt = 0.0;

Food * temp = list;

while (temp!= NULL)
{
if (temp->foodGroup[0] == 'v')
{
vAmnt+=1.0;
vegCount+=temp->calories;
}
else if (temp->foodGroup[0] == 'm')
{
mAmnt+=1;
meatCount+=temp->calories;
}
else if (temp->foodGroup[0] == 'd')
{
dAmnt+=1;
dairyCount +=temp->calories;
}
else if (temp->foodGroup[0] == 'g')
{
gAmnt+=1;
grainsCount+=temp->calories;
}
else if (temp->foodGroup[0] == 'f')
{
fAmnt+=1;
fatCount+=temp->calories;
}
temp = temp->next;
}
printf("%.2f\n", calcAvg(vegCount,vAmnt));
printf("%.2f\n", calcAvg(meatCount,mAmnt));
printf("%.2f\n", calcAvg(dairyCount,dAmnt));
printf("%.2f\n", calcAvg(grainsCount,gAmnt));
printf("%.2f\n", calcAvg(fatCount,fAmnt));

}

double calcAvg(double x, double y)
{
double ans = x/y;
if (ans != ans) return 0.0;
return ans;
}

void run

Solution

Reducing repetition

The big if statement in printAvgCalPerGroup is a bit repetitive (as you pointed out). Imagine if you increased the number of food groups. You would have to copy/paste 4 places for each food group you added.

First, I would suggest using arrays for the counts and amounts of each foodgroup. Then you only have to convert the letter such as 'v' into an array index.

Second, if the food groups could possibly change, I would suggest using an enum to keep track of them. This is definitely optional, and you could use hardcoded numbers instead (e.g. 'v' -> 0, 'm' -> 1, etc).

Third, a small change to make would be to track the amounts using ints instead of doubles, since the amounts always increase by 1 only.

Here is what your code would look like using arrays and enums:

typedef enum FoodGroup {
    VEG,
    MEAT,
    DAIRY,
    GRAINS,
    FAT,
    FOODGROUP_MAX
} FoodGroup;

void printAvgCalPerGroup(Food * list)
{
    double    count[FOODGROUP_MAX] = {0};
    int       amnt [FOODGROUP_MAX] = {0};
    FoodGroup group;

    Food * temp = list;

    while (temp!= NULL) {
        switch (temp->foodGroup[0]) {
            case 'v': group = VEG;           break;
            case 'm': group = MEAT;          break;
            case 'd': group = DAIRY;         break;
            case 'g': group = GRAINS;        break;
            case 'f': group = FAT;           break;
            default : group = FOODGROUP_MAX; break;
        }
        if (group calories;
        }
        temp = temp->next;
    }

    for(group=0; group<FOODGROUP_MAX; group++)
        printf("%.2f\n", calcAvg(count[group], amnt[group]));
}


If you don't need the enum, just replace VEG with 0, MEAT with 1, etc.

Code Snippets

typedef enum FoodGroup {
    VEG,
    MEAT,
    DAIRY,
    GRAINS,
    FAT,
    FOODGROUP_MAX
} FoodGroup;

void printAvgCalPerGroup(Food * list)
{
    double    count[FOODGROUP_MAX] = {0};
    int       amnt [FOODGROUP_MAX] = {0};
    FoodGroup group;

    Food * temp = list;

    while (temp!= NULL) {
        switch (temp->foodGroup[0]) {
            case 'v': group = VEG;           break;
            case 'm': group = MEAT;          break;
            case 'd': group = DAIRY;         break;
            case 'g': group = GRAINS;        break;
            case 'f': group = FAT;           break;
            default : group = FOODGROUP_MAX; break;
        }
        if (group < FOODGROUP_MAX) {
            amnt [group]++;
            count[group] += temp->calories;
        }
        temp = temp->next;
    }

    for(group=0; group<FOODGROUP_MAX; group++)
        printf("%.2f\n", calcAvg(count[group], amnt[group]));
}

Context

StackExchange Code Review Q#85710, answer score: 8

Revisions (0)

No revisions yet.