1
\$\begingroup\$

Made some adjustments to the previous code thanks to the helpful and thoughtful review by @G.Sliepen.

Problem statement: This program should ask for the total number of shops that will be visited. At each shop, ask for the number of ingredients that need to be purchases. For each ingredient, ask for the price. Keep track of the total for the order so that you can write it down before leaving the shop. This program should also track with order was the cheapest and which shop the cheapest order was at.

Original code: Find the cheapest order placed out of all of the stores visited

1st adjustments to the original code: Adjustments based on reviews #1; Find the cheapest order placed out of all of the stores visited

Any and all hints, tips, tricks, advice, suggestions are greatly appreciated!

enter image description here

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdbool.h>

int read_positive_int(const char* prompt)
{
   printf("%s", prompt);

    int n;
    while (true)
    {
        if (scanf("%d", &n) != 1)
        {
            fprintf(stderr, "Invalid input!\n");
            return 1;
        }
    return n;
    }
}

float read_real_positive(const char* prompt)
{
    printf("%s", prompt);

    float m;
    while (true)
    {
        if (scanf("%f", &m) != 1)
        {
            fprintf(stderr, "Invalid input!\n");
            return 1;
        }
    return m;
    }
}
int main(void)
{
    int num_shops = read_positive_int("How many shops will be visited? ");

    float total_cost[num_shops];

    float cheapest_order;
    int cheapest_shop = 1;

    for (int i = 0; i < num_shops; i++)
    {
        printf("You are at shop #%d.\n", i+1);
        int num_ingredients = read_positive_int("How many ingredients are needed? ");

        float cost_ingredient[num_ingredients];

        total_cost[i] = 0;

        for (int j = 0; j < num_ingredients; j++)
        {
            printf("What is the cost of ingredient #%d", j+1);
            cost_ingredient[j]= read_real_positive("? ");
            total_cost[i] += cost_ingredient[j];
        }
        printf("The total cost at shop #%d is $%.2f.\n", i+1, total_cost[i]);

        if (i == num_shops - 1)
        {
            cheapest_order = total_cost[0];

            for (int k = 1; k < num_shops; k++)
            {
                if (total_cost[k] < cheapest_order)
                {
                    cheapest_order = total_cost[k];
                    cheapest_shop = k + 1;
                }
            }
        }
    }
    printf("The cheapest  order  was  at  shop #%d, and the total cost of the order was $%0.2f\n", cheapest_shop, cheapest_order);
    return 0;
}![enter image description here](https://i.sstatic.net/GcAtS.jpg)
\$\endgroup\$
4
  • \$\begingroup\$ Have you tested this? It doesn't seem correct to me. \$\endgroup\$
    – Edward
    Commented Oct 5, 2018 at 14:19
  • \$\begingroup\$ @Edward I posted a picture! Try again please :) I just updated it. \$\endgroup\$ Commented Oct 5, 2018 at 14:28
  • 1
    \$\begingroup\$ You've made ten revisions to the question in the hour since you originally posted it. While that isn't necessarily a problem, it does suggest that maybe it would be appropriate to make sure it's the best code you can write before you ask others to spend their time reviewing it. It's a good way to get the most out of a code review. \$\endgroup\$
    – Edward
    Commented Oct 5, 2018 at 14:48
  • \$\begingroup\$ @Edward I wanted to make the code shorter. It now searches for the minimum integer in the array instead of the code I had before which used a slightly different method but had more lines, I believe. I tested it before I posted and was under the impression that it did indeed work, but I may in fact be incorrect. Nevertheless, it irrevocably works now! \$\endgroup\$ Commented Oct 5, 2018 at 14:51

1 Answer 1

2
\$\begingroup\$

I see some things that may help you further improve your program.

Eliminate arrays by carefully rethinking the problem

At the end of the program, all that is needed is the cheapest_order and the cheapest_shop. All of the arrays could be eliminated by simply keeping, as each shop total is calculated, the current cheapest order. There is no need to keep any data from a shop that is not cheapest, nor any reason to store the ingredient costs in an array.

Fix the bugs

If the scanf fails, as it does if the user inputs a letter instead of a number, the program acts as if the number 1 had been entered, and the user gets no chance to correct it. Also, when I entered -1 for the number of shops to be visited, I got this result:

How many shops will be visited? -1
The cheapest  order  was  at  shop #1, and the total cost of the order was $81082112.00

Test your code thoroughly

One way to test the code is to simply try it multiple times. A better way is to automate the process by writing a test driver. There are many ways to do this; one simple way is to manually create a few files with test input and then feed them to the program.

Don't use floating point for money

There is a problem using floating point (that is, float or double types) to represent money values. See this question for a thorough explanation for why that's the case.

An alternative is to keep a number of cents as an integer value internally. For more depth about floating point issues, I'd recommend the excellent article "What every computer scientist should know about floating-point arithmetic" by David Goldberg.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.