Discount on most expensive item and return total price as integer rounded down if total is float num-CodePudding

I was given the following program to code in a test. I wanted to know if my solution is correct or if there's a better way to do it. I can't find a better to code it.

Question

You work for a shop that wishes to give a discount of discount% to the most expensive item purchased by a given customer during the sales period. You are tasked by the shop owner to implement a function calculate_total_price(prices,discount) which takes the list of prices of products purchased by a customer and the percentage discount as parameters and returns the total purchased price as an integer (rounded down if total price is a float number).

Proposed Solution

 // Online C   compiler to run C   program online
    #include <iostream>
    #include <vector>
    #include <algorithm>
    #include <string>
    #include <math.h>
    
    int calculate_total_price(std::vector<int> prices, int discount){
        double totalSum=0.0;
        //Apply discount on most expensive Item
        std::sort(prices.begin(),prices.end());
        double mostExpensiveItem = prices[prices.size()-1] - prices[prices.size()-1]*discount/100.0;
        
       //Compute the sum with the exception of the last item, where the discount is applied and add discounted last item
        for(int i = 0; i <  prices.size() - 1;i  ){totalSum =prices[i];}
        totalSum =mostExpensiveItem;
       
        //check if total is float. If so, return the floor
        if(abs(totalSum-int(totalSum))>0){
            return floor(totalSum);
        }
        return totalSum;
        
    }
    
    int main() {
       std::vector<int> prices = {10,20,131,40,30};
       int discount = 20;
       std::cout << calculate_total_price(prices,discount);
    
        return 0;
    }

CodePudding user response：

On first sight it seems like your solution should work, assuming the list is not empty as mentioned by Drew Dormann.

However, you can certainly simplify it:

You don't need a sorted list, you simply need the largest value.
There's no need to explicitly floor the result, since the return value of the function is an int, so if you return a double it will automatically be truncated, which is equivalent to flooring for positive values.
You shouldn't take the prices argument as a value, that's going to copy it, which is no good.

So you'd get something like:

int calculate_total_price(std::vector<int> const & prices, int discount){
  double total = std::accumulate(prices.begin(), prices.end(), 0.);

  // Apply discount on most expensive Item
  auto it = std::max_element(prices.begin(), prices.end());
  if (it != prices.end()) {
    total -= *it * discount / 100.;
  }
       
  return total;
}

Note that I'm avoiding for loops, and am instead using functions whose name says what they do. I find this tends to make code more readable, because there's no need to look at the body of a for loop to figure out what's going on.

CodePudding user response：

Here's a simpler way to do it, since you are looping through, just find the biggest discount in that loop:

int calculate_total_price(const std::vector<int>& prices, int discountPct)
{
    double totalSum=0.0, discountPrice=0.0;
    for (auto price : prices) {
        totalSum  = price;

        // discount on the most expensive item is the same as biggest discount on any single item
        auto thisDiscount = price * 0.01 * discountPct;
        if (thisDiscount > discountPrice) discountPrice = thisDiscount;
        // one-liner:
        // discountPrice = std::max(discountPrice, price * 0.01 * discountPct);
    }
    return totalSum - discountPrice; // conversion from double to int already does round toward zero
}

Thanks @AVH for pointing out the useless vector copy.

CodePudding user response：

I'm late in the game but I'll throw in another solution using std::accumulate from <numeric> that finds the most expensive item while accumulating:

#include <algorithm>
#include <numeric>
#include <vector>

int calculate_total_price(const std::vector<int>& prices, int discount) {
    int most_expensive = 0;

    int total = std::accumulate(prices.begin(), prices.end(), 0,
                    [&most_expensive](int current_total, int item_price) {
                        most_expensive = std::max(most_expensive, item_price);
                        return current_total   item_price;
                    });
    
    return total - most_expensive * 0.01 * discount;
}