Calculate amount of necessary ingredients

Question

I have started to learn C and decided to recreate my bakery task in C.
As I am new to the language, I am unsure if I have approached the task in the right way using structs. Feedback on the style of the code would also be appreciated.

#include <stdio.h>

double cup_ingredients[4] = {4.0,0.1,12.0,14.0}; // Amount of each ingredient for 1 cupcake = {Butter, eggs, flour, sugar}
double lemon_ingredients[4] = {80.0,4.5,240.0,300.0}; // Amount of each ingredient for 1 lemon cake = {Butter, eggs, flour, sugar}
double total[4];
double cup_req;
double lemon_req;

struct Bags {
    int big_bag;
    int med_bag;
    int small_bag;
};

void calc_bag(double total_ingredient, struct Bags* bag_sizes, struct Bags* type);

int main() {
    printf("How many cupcakes would you like? ");
    scanf("%lf", &cup_req);

    for (int x = 0; x<cup_req; x++){ // For the number of cupcakes required:
        for (int y = 0; y<4; y++){ // For each ingredient
            total[y] += cup_ingredients[y]; // Add the amount of each ingredient to the total amount of that ingredient
        }
    }

    printf("How many lemon cakes would you like? ");
    scanf("%lf", &lemon_req);

    for (int x = 0; x<lemon_req; x++){ // For the number of lemon cakes:
        for (int y = 0; y<4; y++){ // For each ingredient
            total[y] += lemon_ingredients[y]; // Add the amount of each ingredient to the total amount of that ingredient
        }
    }

    //Structs for the amount of each ingredient a bag can hold
    struct Bags Butter_size = {.big_bag = 500, .med_bag = 250, .small_bag = 125};
    struct Bags Egg_size = {.big_bag = 12, .med_bag = 10, .small_bag = 6};
    struct Bags Flour_size = {.big_bag = 750, .med_bag = 500, .small_bag = 250};
    struct Bags Sugar_size = {.big_bag = 600, .med_bag = 400, .small_bag = 200};

    //Set the bags required to 0
    struct Bags Butter_req = {0,0,0};
    struct Bags Egg_req = {0,0,0};
    struct Bags Flour_req = {0,0,0};
    struct Bags Sugar_req = {0,0,0};

    //Calculate the amount of each ingredient bag required
    calc_bag(total[0], &Butter_size, &Butter_req);
    calc_bag(total[1], &Egg_size, &Egg_req);
    calc_bag(total[2], &Flour_size, &Flour_req);
    calc_bag(total[3], &Sugar_size, &Sugar_req);

    printf("\nButter: %d large bags, %d medium bags, %d small bags.", Butter_req.big_bag, Butter_req.med_bag, Butter_req.small_bag);
    printf("\nEgg: %d large bags, %d medium bags, %d small bags.", Egg_req.big_bag, Egg_req.med_bag, Egg_req.small_bag);
    printf("\nFlour: %d large bags, %d medium bags, %d small bags.", Flour_req.big_bag, Flour_req.med_bag, Flour_req.small_bag);
    printf("\nSugar: %d large bags, %d medium bags, %d small bags.", Sugar_req.big_bag, Sugar_req.med_bag, Sugar_req.small_bag);
}

void calc_bag(double total_ingredient, struct Bags* bag_sizes, struct Bags* type){
    while (total_ingredient > 0){
        if (total_ingredient > bag_sizes->big_bag) {
            type->big_bag++;
            total_ingredient -= bag_sizes->big_bag;
        }
        else if (total_ingredient > bag_sizes->med_bag) {
            type->med_bag++;
            total_ingredient -= bag_sizes->med_bag;
        }
        else if (total_ingredient > bag_sizes->small_bag) {
            type->small_bag++;
            total_ingredient -= bag_sizes->small_bag;
        }
        else {
            type->small_bag++;
            total_ingredient = 0;
        }
    }
}

Answer 1

I see some things that may help you improve your code.

Eliminate function prototypes by ordering

If you put the calc_bag implementations above main in the source code, you don't need the function prototype.

Avoid the use of global variables

I see that cup_ingredients and lemon_ingredients, etc. are declared as global variables rather than as local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. In this case, these should all be in main rather than global.

Initialize variables

Global variables are initialized for you (to 0 for numeric variables), but local variables are not. For that reason, you should also get into the habit of initializing variables, ideally when they're declared. For example:

double total[4] = {0.0, 0.0, 0.0, 0.0};
double cup_req = 0.0;
double lemon_req = 0.0;

Use const where practical

The ingredients lists cup_ingredients and lemon_ingredients, as well as the bag capacities Butter_size, etc. should all be constant. For that reason, they should all be declared static const as in:

static const struct Bags Butter_size = {.big_bag = 500, .med_bag = 250, .small_bag = 125};

Then the calc_bag function should be this:

void calc_bag(double total_ingredient, const struct Bags* bag_sizes, struct Bags* type);

Simplify by using a typedef

The code you have isn't wrong, but it's often convenient to use a typedef for structures that are used frequently. In this case, I'd suggest that your Bags structure could be this:

typedef struct bags_s {
    int big_bag;
    int med_bag;
    int small_bag;
} Bags;

Then instead of writing struct Bags everywhere, you can simply write Bags.

Prefer multiplication to iteration

Especially when using floating point numbers, it's most often better to multiply than to use iteration. For example, the code currently has this:

printf("How many cupcakes would you like? ");
scanf("%lf", &cup_req);

for (int x = 0; x<cup_req; x++){ 
    for (int y = 0; y<4; y++){ 
        total[y] += cup_ingredients[y]; 
    }
}

That could be replaced by this:

for (int i = 0; i < 4; ++i) {
    total[i] += cup_req * cup_ingredients[i];
}

Similarly, your calc_bag function could use division rather than iteration.

Break up the code into smaller functions

The main function is quite long and does a series of identifiable steps. Rather than having everything in one long function, it would be easier to read and maintain if each discrete step were its own function. I'd be inclined to divide it into separate input, calculation, and output stages, each with the appropriate function.

Eliminate "magic values"

The value 4 is sprinkled through the code, but it really ought to be a named constant instead. I'd give it a meaningful name like this:

#define INGREDIENT_COUNT 4

Rethink your data structures

The only difference between the cup_ingredients and lemon_ingredients is the name. They're parallel structures. This relationship could be made more clear by defining another structure which includes the name. One might write it like this:

typedef struct recipe_s {
    char *name;
    double ingredients[INGREDIENT_COUNT];
} Recipe;

With that structure in place, one might rewrite main like this:

int main() {
    static const char *ingredient_name[INGREDIENT_COUNT] = { 
        "butter", "eggs", "flour", "sugar",
    };
    static const Recipe recipes[] = {
        { "cupcakes", {4.0,0.1,12.0,14.0} },
        { "lemon cakes", {80.0,4.5,240.0,300.0} },
    };
    static const int recipe_count = sizeof(recipes) / sizeof(recipes[0]);
    Recipe total = { "total", {0.0, 0.0, 0.0, 0.0} };

    for (int i = 0; i < recipe_count; ++i) {
        double qty;
        printf("How many %s would you like? ", recipes[0].name);
        scanf("%lf", &qty);
        for (int j = 0; j < INGREDIENT_COUNT; ++j) {
            total.ingredients[j] += qty * recipes[i].ingredients[j];
        }
    }

    //Structs for the amount of each ingredient a bag can hold
    static const Bags capacity[INGREDIENT_COUNT] = {
        { 500, 250, 125}, // butter
        {  12,  10,   6}, // eggs
        { 750, 500, 250}, // flour
        { 600, 400, 200}, // sugar
    };

    // make a shopping list
    Bags shopping_list[INGREDIENT_COUNT] = {
        {0, 0, 0},  // butter
        {0, 0, 0},  // eggs
        {0, 0, 0},  // flour
        {0, 0, 0},  // sugar
    };

    //Calculate the amount of each ingredient bag required
    for (int i = 0; i < INGREDIENT_COUNT; ++i) {
        calc_bag(total.ingredients[i], &capacity[i], &shopping_list[i]);
        printf("%s: %d large bags, %d medium bags, %d small bags.\n", 
            ingredient_name[i],
            shopping_list[i].big_bag, 
            shopping_list[i].med_bag, 
            shopping_list[i].small_bag 
        );
    }
}

I'll leave it to you to divide that into smaller functions, but it should help you get an idea of how to write better C.

Other enhancements

I would be very disappointed if my grocer actually handed me a "bag of eggs." Instead, the common quantities for different things have different units of measure such as a "dozen" or a "kilogram" or a "pound". Using a similar idea of associating a name with constants (as with the recipes shown above), you might want to associate units of measure with each kind of ingredient.

Also, our lemon cake does not appear to have lemon as an ingredient, which makes it a somewhat less appealing confection. Consider adding the ability to create arbitrary lists of named ingredients, consolidating them into a shopping list as above.

Answer 2

I don't know C, never mind idiomatic C. And so this is more a high-level review of your code. Then a review that says what good C code is.

• Your comments say what the code is doing. Anyone that knows how to program, can tell what your code is doing. And so these are redundant, likely to become outdated if your code changes, and generally clutter up the code. I recommend removing them.
• Why are total, cup_req and lemon_req globally defined? They should be defined in main. You want to keep items to have the smallest scope possible.
• Why are Butter_size, Egg_size, etc. not globally defined? Whilst I don't know how to make them constants, they should probably become global constants.

• The way you build total is awkward. Currently it's:

  printf("How many cupcakes would you like? ");
  scanf("%lf", &cup_req);
  
  for (int x = 0; x<cup_req; x++){ // For the number of cupcakes required:
      for (int y = 0; y<4; y++){ // For each ingredient
          total[y] += cup_ingredients[y]; // Add the amount of each ingredient to the total amount of that ingredient
      }
  }
  
  printf("How many lemon cakes would you like? ");
  scanf("%lf", &lemon_req);
  
  for (int x = 0; x<lemon_req; x++){ // For the number of lemon cakes:
      for (int y = 0; y<4; y++){ // For each ingredient
          total[y] += lemon_ingredients[y]; // Add the amount of each ingredient to the total amount of that ingredient
      }
  }
  

  However if you flip the for loops so it loops through the ys first, then the xs, you can combine them. However, you should be able to tell that you can just multiply. This means you can use:

  printf("How many cupcakes would you like? ");
  double cup_req;
  scanf("%lf", &cup_req);
  printf("How many lemon cakes would you like? ");
  double lemon_req;
  scanf("%lf", &lemon_req);
  
  double total[4] = {0, 0, 0, 0};
  for (int y = 0; y<4; y++){
      total[y] += cup_req * cup_ingredients[y];
      total[y] += lemon_req * lemon_ingredients[y];
  }
  

• Having to define Butter_req and co. would be annoying to work with. Why not do that in calc_bag, and return it?

• The function calc_bag suffers from the same performance problems as stated in my previous answer. The way that it currently is, causes it to run in \$O(n)\$ time. Where you can easily make it run in \$O(1)\$ time. Just make a divmod function.
• I don't know if you can, but you may want to look into merging all the Butter_req calls and printf calls together. As a simple loop that looped through the different sizes would be easy to make and make your code DRY.

And so I'd change to use something like:

#include <stdio.h>


struct Bags {
    int big_bag;
    int med_bag;
    int small_bag;
};

struct DivMod {
    int result;
    double remainder;
};

// Ingredients per item {butter, eggs, flour, sugar}
double cup_ingredients[4] = {4.0, 0.1, 12.0, 14.0};
double lemon_ingredients[4] = {80.0, 4.5, 240.0, 300.0};

struct Bags Butter_size = {.big_bag = 500, .med_bag = 250, .small_bag = 125};
struct Bags Egg_size = {.big_bag = 12, .med_bag = 10, .small_bag = 6};
struct Bags Flour_size = {.big_bag = 750, .med_bag = 500, .small_bag = 250};
struct Bags Sugar_size = {.big_bag = 600, .med_bag = 400, .small_bag = 200};


struct DivMod divmod(double a, double b) {
    int result = (int) (a / b);
    double remainder = a - result * b;
    struct DivMod ret = {result, remainder};
    return ret;
}


struct Bags calc_bag(double total_ingredient, struct Bags* bag_sizes);
struct Bags calc_bag(double total_ingredient, struct Bags* bag_sizes){
    struct Bags type = {0,0,0};

    struct DivMod big_result = divmod(total_ingredient, bag_sizes->big_bag);
    type.big_bag = big_result.result;
    total_ingredient = big_result.remainder;

    struct DivMod med_result = divmod(total_ingredient, bag_sizes->med_bag);
    type.med_bag = med_result.result;
    total_ingredient = med_result.remainder;

    struct DivMod small_result = divmod(total_ingredient, bag_sizes->small_bag);
    type.small_bag = small_result.result;
    total_ingredient = small_result.remainder;

    if (total_ingredient > 0) {
        type.small_bag += 1;
    }

    return type;
}


int main() {
    printf("How many cupcakes would you like? ");
    double cup_req;
    scanf("%lf", &cup_req);
    printf("How many lemon cakes would you like? ");
    double lemon_req;
    scanf("%lf", &lemon_req);

    double total[4] = {0, 0, 0, 0};
    for (int y = 0; y<4; y++){
        total[y] += cup_req * cup_ingredients[y];
        total[y] += lemon_req * lemon_ingredients[y];
    }

    struct Bags Butter_req = calc_bag(total[0], &Butter_size);
    struct Bags Egg_req = calc_bag(total[1], &Egg_size);
    struct Bags Flour_req = calc_bag(total[2], &Flour_size);
    struct Bags Sugar_req = calc_bag(total[3], &Sugar_size);

    printf("\nButter: %d large bags, %d medium bags, %d small bags.", Butter_req.big_bag, Butter_req.med_bag, Butter_req.small_bag);
    printf("\nEgg: %d large bags, %d medium bags, %d small bags.", Egg_req.big_bag, Egg_req.med_bag, Egg_req.small_bag);
    printf("\nFlour: %d large bags, %d medium bags, %d small bags.", Flour_req.big_bag, Flour_req.med_bag, Flour_req.small_bag);
    printf("\nSugar: %d large bags, %d medium bags, %d small bags.", Sugar_req.big_bag, Sugar_req.med_bag, Sugar_req.small_bag);
}