It's maybe unnecessary for this little code, but I'd get into the habit of writing different functions for the different steps of your algorithm. You'd notice that breaking it in smaller functions gives readability, and the opportunity to expand the functionality easily.
For example, I broke your code to smaller functions, which let me add some error checking (removing the "pedantic" warnings mentioned by @Caridorc, checking that number of flowers is not greater than the array size, ...). Adding error checking would have been much more difficult (harder to read) if it had been done directly in the main function.
Writing smaller function also makes the code more self-documenting: by choosing good function names, you're really saying what the code is doing now.
Initializing to 0 with memset
is a bit old school. You can 0-init arrays this way:
int array[A_GOOD_DEFINE] = {0};
Defensive programming (as mentioned by @Caridorc) is very valuable. Always use return values (look up perror
). It's more code, most of your function calls are made as if statements, but it's really worth it, a very good habit to get into. I forgot how to use a debugger since I starting coding that way.
My example of user_input_get()
below looks a bit silly, partly because of the bracing style (it's only an opinion, but since defensive programming is doing nearly everything in if
statements, there are a lot of rows with only braces. Especially if you don't want to have braceless if
bodies). To solve this it could be broken down even further: user_input_sizes_get()
and user_input_costs_get()
. Left for you to do :)
Prompt the user for input with printf
, and what should be entered next. Right now the program just waits without asking anything, it looks like it froze.
With these changes applied (again, break the input function further). I believe the main function shows a clearer overview.
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define COSTS_ARRAY_SIZE (100u)
int comp_desc(const void * a, const void * b)
{
int * aPtr = (int*)a;
int * bPtr = (int*)b;
return *bPtr - *aPtr;
}
int user_input_get(int *needed, int *numFriends, int *costs, size_t size)
{
errno = 0;
if (printf("Enter number of needed flowers, then number of friends: ") < 0)
{
perror("printf");
return -1;
}
int n_read_values = scanf("%d %d", needed, numFriends);
// The following inspired by the man page for scanf.
if (n_read_values != 2)
{
if (errno != 0)
{
perror("scanf");
}
else
{
fprintf(stderr, "No matching characters\n");
}
return -1;
}
else
{
if (*needed > COSTS_ARRAY_SIZE)
{
fprintf(stderr, "Too many flowers needed\n");
return -1;
}
for (int i = 0; i < *needed; ++i)
{
if (printf("Cost of flower %d: ", i + 1) < 0)
{
perror("printf");
return -1;
}
n_read_values = scanf("%d", &costs[i]);
if (n_read_values != 1)
{
if (errno != 0)
{
perror("scanf");
}
else
{
fprintf(stderr, "No matching characters\n");
}
return -1;
}
}
}
return 0;
}
int money_spent_get(int needed, int costs[], int numFriends)
{
int spent = 0;
int bought = 0;
int multiplier = 1;
for (int i = 0; i < needed; ++i)
{
spent += costs[i] * multiplier;
++bought;
multiplier = (bought / numFriends) + 1;
}
return spent;
}
int main()
{
int flowersNeeded, numFriends;
int flowerCosts[COSTS_ARRAY_SIZE] = {0}; // Better init than memset()
if (user_input_get(&flowersNeeded, &numFriends, flowerCosts, COSTS_ARRAY_SIZE) < 0)
{
fprintf(stderr, "Error: user_input_get\n");
exit(EXIT_FAILURE);
}
qsort(flowerCosts, flowersNeeded, sizeof(int), comp_desc);
int moneySpent = money_spent_get(flowersNeeded, flowerCosts, numFriends);
printf("%d\n", moneySpent);
exit(EXIT_SUCCESS);
}