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

HackerRank "Flowers" challenge

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

Problem

I'm fairly new to C and wanted a general review of this code. It's a solution to this problem.


You and your \$K-1\$ friends want to buy \$N\$ flowers. Flower number \$i\$ has
cost \$c_i\$. Unfortunately the seller does not want just one customer to
buy a lot of flowers, so he tries to change the price of flowers for
customers who have already bought some flowers. More precisely, if a
customer has already bought \$x\$ flowers, he should pay \$(x+1)*c_i\$ dollars
to buy flower number \$i\$.


You and your \$K-1\$ friends want to buy all \$N\$ flowers in such a way that
you spend the least amount of money. You can buy the flowers in any
order.


Input:


The first line of input contains two integers \$N\$ and \$K\$ (\$K \le N\$). The
next line contains \$N\$ space separated positive integers \$c_1,c_2,\dotsc,c_N\$.


Output:


Print the minimum amount of money you (and your friends) have to pay
in order to buy all \$N\$ flowers.

#include 
#include 
#include 

int comp_desc(const void * a, const void * b)
{
    int * aPtr = (int*)a;
    int * bPtr = (int*)b;
    return *bPtr - *aPtr;
}

int main()
{
    int flowersNeeded, numFriends, i;
    scanf("%d %d", &flowersNeeded, &numFriends);
    getchar();
    int flowerCosts[100];
    memset(flowerCosts, 0, sizeof(flowerCosts));
    for (i = 0; i < flowersNeeded; ++i)
    {
        scanf("%d", &flowerCosts[i]);
    }
    qsort(flowerCosts, flowersNeeded, sizeof(int), comp_desc);
    int flowersBought = 0;
    int moneySpent = 0;
    int multiplier = 1;
    for (i = 0; i < flowersNeeded; ++i)
    {
        moneySpent += flowerCosts[i] * multiplier;
        multiplier = (++flowersBought / numFriends) + 1;
    }
    printf("%d\n", moneySpent);
    return 0;
}

Solution

You should declare loop variables as close as possible to the loop

int flowersNeeded, numFriends, i;
// ... some code
for (i = 0; i < flowersNeeded; ++i)


The above code causes inefficiency and puts unecessary mental burden on the reader, you should use (in C99):

int flowersNeeded, numFriends;
// ... some code
for (int i = 0; i < flowersNeeded; ++i)


Avoid modifyng in place and assigning at the same time

multiplier = (++flowersBought / numFriends) + 1;


The above line is too much compact, usually you assign to a variable xor you increment a variable in place, I would explode it in two lines:

flowersBought++;
multiplier = (flowersBought / numFriends) + 1;


Sorry for being pedantic

The very same compiler you use to generate the binaries doubles as a code analysis tool:

gcc flowers.c -Wall -pedantic


Warns you about:

h.c: In function ‘main’:
h.c:15:5: warning: ignoring return value of ‘scanf’, declared with attribute warn_unused_result [-Wunused-result]
     scanf("%d %d", &flowersNeeded, &numFriends);
     ^
h.c:21:9: warning: ignoring return value of ‘scanf’, declared with attribute warn_unused_result [-Wunused-result]
         scanf("%d", &flowerCosts[i]);
         ^

Code Snippets

int flowersNeeded, numFriends, i;
// ... some code
for (i = 0; i < flowersNeeded; ++i)
int flowersNeeded, numFriends;
// ... some code
for (int i = 0; i < flowersNeeded; ++i)
multiplier = (++flowersBought / numFriends) + 1;
flowersBought++;
multiplier = (flowersBought / numFriends) + 1;
gcc flowers.c -Wall -pedantic

Context

StackExchange Code Review Q#92581, answer score: 3

Revisions (0)

No revisions yet.