2016-11-13 11 views
1

Teil eines Programms, an dem ich arbeite, implementiert eine Funktion, die das Paketgewicht als Argument berücksichtigt und die Versandkosten basierend auf diesem Gewicht berechnet. Die Kriterien für die Kosten/lb ist wie folgt:C++ Berechnung der Versandkosten basierend auf Gewicht

 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 

habe ich eine if-else if-if, um die Berechnungen zu machen, aber das Gefühl, es ist ein bisschen wiederholend:

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; 

Also, die Frage ist ... ist dies der beste Weg, um diese Aufgabe zu erfüllen, oder sollte ich nach einer anderen Lösung suchen?

+3

Was Sie haben, ist in Bezug auf gute Praxis makellos. Es gibt nichts Wiederholendes, was Ihre Verwendung von if-else if-else betrifft. – VHS

+0

Außerdem ist hier nichts rekursiv. –

+0

Angenommen, dies sind die einzigen Ebenen, das sieht gut aus. – Qix

Antwort

2

Zunächst einmal, Sie Code sieht klar und ok wie es ist.

Natürlich könnten Sie die redundanten Teile der Formeln Deduplizierung durch einen kumulativen Ansatz:

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); 
} 

Nun, das noch weiter vereinfacht werden:

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; 

Für 3 Waagen, es ist wahrscheinlich ok mit dieser synthetischen Formel. Wenn Sie jedoch mehr Flexibilität wünschen, könnten Sie daran denken, einen Vektor von Raten zu durchlaufen, anstatt Konstanten zu verwenden. Dies würde eine variable Anzahl von Skalen ermöglichen. Wenn Sie sicher sind, dass die Formel immer progressiv ist (z. B. "über + neuer Einheitspreis für das, was übertrifft"), dann verwenden Sie den kumulativen Ansatz.

+0

Danke für das Feedback! Dies ist mehr von dem, was ich im Sinn hatte, aber ich konnte es nicht durchdenken. Sehr geschätzt! – jslice

+0

Während Ihre Lösungen vereinfacht sind, gibt es Kompromisse hinsichtlich der Effizienz? Der erste verwendet einen Vergleich weniger Operationen als mein Original, aber der letzte, den Sie gepostet haben, verwendet ein paar Minuten/Max. Wenn die Ratenprogression in einem größeren Maßstab wäre, würde es einen Unterschied machen? – jslice

+0

Sie haben zwar Recht, aber es hängt stark vom Optimierer ab. Zum Beispiel ist mit GCC 6.2 mein [erster Vorschlag] (https://godbolt.org/g/pPQCgO) nur eine Asm-Anweisung weniger als Ihr [Originalcode] (https://godbolt.org/g/sbkze7). Und meine [Ultraslim Funktion] (https://godbolt.org/g/x3wxeA) ist 4 Anweisungen mehr. Die gesamte Ausführungsleistung hängt jedoch nicht nur von der Anzahl der Befehle, sondern auch von der statistischen Verteilung des Gewichts ab (wobei mehr oder weniger Sprünge erforderlich sind). Daher mein Einwand: "Vorzeitige Optimierung ist die Wurzel allen Übels". – Christophe

0

Ich denke, es gibt viele fast identische Zeilen im Code, aber keine echten Duplikate. Wenn Sie weitere Raten hinzufügen, können Sie einfach die falschen Makrodefinitionen kopieren oder die Werte mit der falschen Rate mischen.

Mein Code selbst entfernt die if/else-Replikationen und vermeidet die Notwendigkeit, die korrekte globale Definition zu verwenden. Wenn Sie meinem Code eine neue Rate hinzufügen, fügen Sie einfach einen Raw zur Tabelle hinzu.

Nur eine Idee zu geben, was sonst getan werden kann:

#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); 
} 

Wenn C++ 11 nicht verfügbar ist, können Sie auch normale Funktionen und Funktionszeiger statt lambdas und std :: Funktion verwenden.

Verwandte Themen