2016-11-13 3 views
1

Часть программы, в которой я работаю, реализует функцию, которая принимает вес пакета в качестве аргумента и рассчитывает стоимость доставки на основе этого веса. Критерии затрат/фунт выглядит следующим образом:C++ Расчет стоимости доставки на основе веса

 Package Weight    Cost 
     --------------    ---- 
     25 lbs & under    $5.00 (flat rate) 
     26 - 50 lbs     above rate + 0.10/lb over 25 
     50 + lbs     above rate + 0.07/lb over 50 

я использовал если-если еще-если сделать расчеты, но чувствует, что его немного повторяющийся:

const int TIER_2_WEIGHT = 25; 
const int TIER_3_WEIGHT = 50; 

const float TIER_1_RATE = 5.00; 
const float TIER_2_RATE = 0.10; 
const float TIER_3_RATE = 0.07; 

float shipPriceF; 


if(shipWeightF <= TIER_2_WEIGHT) 
{ 
    shipPriceF = TIER_1_RATE; 
} 
else if(shipWeightF <= TIER_3_WEIGHT) 
{ 
    shipPriceF = ((shipWeightF - TIER_2_WEIGHT) * TIER_2_RATE) + 
        TIER_1_RATE; 
} 
else 
{ 
    shipPriceF = ((shipWeightF - TIER_3_WEIGHT) * TIER_3_RATE) + 
       ((TIER_3_WEIGHT - TIER_2_WEIGHT) * TIER_2_RATE) + 
        TIER_1_RATE; 
} 

return shipPriceF; 

Таким образом, вопрос ... это лучший способ выполнить эту задачу, или я должен искать другое решение?

+3

То, что у вас есть, является безукоризненным с точки зрения хорошей практики. Нет ничего повторяющегося в отношении использования if-else if-else. – VHS

+0

Кроме того, здесь нет ничего рекурсивного. –

+0

Предполагая, что это единственные уровни, это выглядит отлично. – Qix

ответ

2

Прежде всего, вы код выглядит ясным и нормально, как есть.

Конечно, вы можете дедуплицировать избыточные части формул, используя кумулятивный подход:

float shipPriceF = TIER_1_RATE; // to be paid anyway 

if (shipWeightF > TIER_2_WEIGHT) // add the tier 2 if necessary 
{ 
    shipPriceF += (min(shipWeightF, TIER_3_WEIGHT) - TIER_2_WEIGHT) * TIER_2_RATE; 
} 
if(shipWeightF > TIER_3_WEIGHT) // add the tier 3 if really necessary 
{ 
    shipPriceF += (shipWeightF - TIER_3_WEIGHT) * TIER_3_RATE); 
} 

Ну, это даже может быть упрощена дальше:

float shipPriceF = TIER_1_RATE 
        + max(min(shipWeightF,TIER_3_WEIGHT)-TIER_2_WEIGHT,0) * TIER_2_RATE 
        + max(shipWeightF-TIER_3_WEIGHT,0) * TIER_3_RATE; 

Для 3 шкал, это вероятно, с этой синтетической формулой. Однако, если вы хотите больше гибкости, вы можете подумать о том, чтобы итерации вектора ставок вместо использования констант. Это позволило бы варьировать количество шкал. Если вы уверены, что формула всегда прогрессивна (например, «выше + новая цена за единицу за то, что выходит»), используйте кумулятивный подход.

+0

Спасибо за обратную связь! Это больше того, что я имел в виду, но не мог продумать это. Очень признателен! – jslice

+0

Хотя ваши решения упрощены, будут ли какие-либо компромиссы для эффективности? Первый использует одну менее сравнительную операцию, чем мой оригинал, но последний, который вы опубликовали, использует несколько минут/максимальных значений. Если бы скорость была в большем масштабе, изменилась бы она? – jslice

+0

Вы действительно правы, но это сильно зависит от оптимизатора. Например, с GCC 6.2 мое [первое предложение] (https://godbolt.org/g/pPQCgO) является только одной инструкцией asm меньше вашего [исходного кода] (https://godbolt.org/g/sbkze7). И моя функция [ultraslim] (https://godbolt.org/g/x3wxeA) - еще 4 инструкции. Но общая производительность исполнения зависит не только от количества инструкций, но и от статистического распределения веса (требующего более или менее прыжков). Поэтому, моя привлекательность: «Преждевременная оптимизация - это корень всего зла». – Christophe

0

Я думаю, что в коде много идентичных строк, но не реальных дубликатов. Если вы добавите больше ставок, вы можете легко скопировать неправильные определения макросов или смешать значения с неправильной скоростью.

Мой код сам удаляет повторы if/else и избегает использования правильного глобального определения. Если вы добавите новый курс в мой код, вы просто добавите необработанное значение в таблицу.

только, чтобы дать представление о том, что еще можно сделать:

#include <iostream> 
#include <functional> 
#include <limits> 

// first we define a entry of a table. This table contains the limit to which the ratio is valid and 
// a function which calculates the price for that part of the weight. 
struct RateTableEntry 
{ 
    double max; 
    std::function<double(double, double)> func; 
}; 

// only to shrink the table width :-) 
constexpr double MAX = std::numeric_limits<double>::max(); 

// and we define a table with the limits and the functions which calculates the price 
RateTableEntry table[]= 
{ 
    // first is flate rate up to 25 
    { 25, [](double , double  )->double{ double ret=      5.00; return ret; }}, 
    // next we have up to 50 the rate of 0.10 (use min to get only the weight up to next limit 
    { 50, [](double max, double weight)->double{ double ret= std::min(weight,max)*0.10; return ret; }}, 
    // the same for next ratio. std::min not used, bedause it is the last entry 
    { MAX, [](double , double weight)->double{ double ret=   weight  *0.07; return ret; }} 
}; 

double CalcRate(double weight) 
{ 
    std::cout << "Price for " << weight; 
    double price = 0; 
    double offset = 0; 
    for (auto& step: table) 
    { 
     // call each step, until there is no weight which must be calculated 
     price+=step.func(step.max- offset, weight); 
     // reduce the weight for that amount which allready is charged for 
     weight-=step.max-offset; 
     // make the table more readable, if not used this way, we have no max values but amount per step value 
     offset+=step.max; 
     if (weight <= 0) break; // stop if all the weight was paid for 
    } 

    std::cout << " is " << price << std::endl; 

    return price; 
} 

int main() 
{ 
    CalcRate(10); 
    CalcRate(26); 
    CalcRate(50); 
    CalcRate(51); 
    CalcRate(52); 
    CalcRate(53); 
} 

Если C++ 11 не доступен, вы можете также использовать обычные функции и указатели на функции вместо лямбды и зЬй :: функции.

Смежные вопросы