patterncMinor
Summarizing calories for a food diary
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
```
#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
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
First, I would suggest using arrays for the counts and amounts of each foodgroup. Then you only have to convert the letter such as
Second, if the food groups could possibly change, I would suggest using an
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:
If you don't need the enum, just replace
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.