Is there a way to shorten this long if / else if return method?

Question

I have this very long method:

public decimal decDiscount(QuoteData quoteData) { if (TotalChapter7(quoteData) >= 7499) return 5300; else if (TotalChapter7(quoteData) >= 7449) return 5300; else if (TotalChapter7(quoteData) >= 7399) return 5250; else if (TotalChapter7(quoteData) >= 7349) return 5200; else if (TotalChapter7(quoteData) >= 7299) return 5200; else if (TotalChapter7(quoteData) >= 7249) return 5150; else if (TotalChapter7(quoteData) >= 7199) return 5100; else if (TotalChapter7(quoteData) >= 7149) return 5100; else if (TotalChapter7(quoteData) >= 7099) return 5050; //... else if (TotalChapter7(quoteData) >= 1199) return 1100; else if (TotalChapter7(quoteData) >= 1149) return 1100; else if (TotalChapter7(quoteData) >= 1099) return 1050; else if (TotalChapter7(quoteData) >= 1049) return 1000; else return 0; } 

which has a repeating pattern that can be illustrated in the following excel sheet:

...... → →

Starting at the highest “Fee” (7499), the fee is reduced by 50 in each application. However, the return (or the "Discounted Price") remains the same for every 100 (two 50 drops in the fees), then 50 drops for one return (one 50 drops in the fee), and then repeats.

As you can see, my method is quite long (which I left between 7049 and 1249). Is there something I can do to cut this down?

Thanks.

Answer 1

You guys are really exaggerating it too much. Any approach to solving a problem using integer arithmetic is a bad idea. See how hard it is for a bunch of really smart people (we are all very smart, aren't we?) To even figure it out first. It is very difficult to detect, it is very difficult to understand, it is difficult to obtain the right, and it is a bitch to maintain.

You need an approach that is easy to understand and easy to maintain. Look at your original post, you have an English description of the rule.

However, the return (or the "Discounted Price") remains the same for every 100 (two 50 drops in the fee), then 50 drops for one return (one 50 drops in the fee), and then repeats.

The code almost writes itself:

 public int GetFeeFromQuoteData(QuoteData quoteData) { int fee = 5300; int difference = 7449 - TotalChapter7(quoteData); bool isTwoStep = true; while (difference > 0) { if (isTwoStep) { difference -= 50; } else { difference -= 100; } fee -= 50; isTwoStep = !isTwoStep; } return fee; } 

Answer 2

These prices and discounts are DATA! Data should never be compiled into code

I wouldn’t collect drops in the code, Id builds the basic pricing structure (they can play well with falling prices, but they are less likely to change the overall pricing structure).

I have prices and discounts in an easily changeable place (for example, database, xml file) that reflects this structure.

 public class Pricing { private List<Tuple<decimal, decimal>> pricePoints= new List<Tuple<int, decimal>> discountRanges(); public Pricing() { // These hard coded values would be replaced by logic to load from file. pricePoints.Add(Tuple.Create(7499, 5300)); pricePoints.Add(Tuple.Create(7399, 5250)); pricePoints.Add(Tuple.Create(7349, 5200)); pricePoints.Add(Tuple.Create(7249, 5150)); . . . pricePoints.Add(Tuple.Create(1049, 1000)); } public decimal GetDiscount(QuoteData quoteData) { var price = TotalChapter7(quoteData); foreach(var point in pricePoints) { if(price >= point.Item1) return point.Item2; } // If we're here it implies there were no matching points return 0; } } 

If you create drops in the code and they change drops, you must change the code.
Put the data in a file, upload the file once at runtime, and they can change prices, and you just need to change the file.

Return to comment "This is obviously a business rule."

Excluding discounts at the point of sale (2 for one offer, 10% of this product, etc.), there are three ways to calculate the discount on the total fee or fee (any of which can be combined with the preferred discount for customers).

• Flat percentage (e.g. 10% always)
• Different percentages for different price points.
• Different flat prices for different price points (this is what we see in the question)

Which of them (or those) that the client decides to use is a business rule and yes, the rule must be presented in code.

However, no matter which rule is used, the actual values ​​are data, and should never be in the code (except in exceptional circumstances).

Answer 3

Perhaps this will help:

  List<Tuple<int, int>> _FeeToPrice = new List<Tuple<int, int>> { new Tuple<int,int>(7499,5300), new Tuple<int,int>(7399,5250), ... new Tuple<int,int>(1049,1000) }; public decimal decDiscount(QuoteData quoteData) { var processedQuoteData = TotalChapter7(quoteData); var tuple = _FeeToPrice.FirstOrDefault(x => processedQuoteData >= x.Item1); if (tuple != null) return tuple.Item2; return 0; } 

Edit: The _FeeToPrice structure can be loaded from a file or database or some other source, which will simplify the correction of the return value

Answer 4

The working formula calculated analytically is as follows:

 int calcDiscount(int p) { int s = (7500/50) - (p+1) / 50; int k = s / 3; int j = s % 3; return 5300 - 100*k - (j == 2 ? 50 : 0) } 

Working test file (in Java):

 for (p = 7499; p >= 1000; p -= 50) { System.out.println (p+ " " + calcDiscount(p)); } 

Explanation: first you find how many steps from the maximum (7499) relate to the current price, then you know that you must reset the discounted value to 100 every three steps, but if you are at the last step of the current triple, you must refuse it yet by 50.

Answer 5

Pseudocode below:

 Dictionary<Int, Int> getDiscounts(int startFee, int startDiscount, int endFee) { Dictionary <Int, Int> quoteDictionary = new Dictionary<Int, Int> (); for(int i = 0; i++; startFee >= endFee) { startFee -= 50; if(i != 0) { startDiscount -= 50; } if(i == 2) { i = -1; } quoteDictionary[startFee] = startDiscount; } return quoteDictionary; } 

You can call it like this:

 Dictionary <Int, Int> prices = getDiscounts(7499, 5300, 1049); int quote = TotalChapter7(quoteData); int roundedQuote = quote - ((quote % 50) + 1); int discountedFee = prices[roundedQuote]; 

Answer 6

I don't know if this will work for you, but you can use the dictionary and sort by key in descending order, as in the above example.

I have not tested it, but something like this might work:

 public int GetNumber(int value) { //initialize a dictionary to hold pairs of numbers var ranges = new SortedDictionary<int, int> { { 25, 250 }, { 50, 500 }, { 75, 750 } }; //sort the dictionary in descending order and return the first value //that satisfies the condition return ranges.OrderByDescending(p => p.Key) .FirstOrDefault(p => value >= p.Key).Value; } 

Answer 7

try using a dictionary, try to make an example, but I do not often use C #, so fix everything that is not correct in the code, just try to get the idea:

 public decimal decDiscount(QuoteData quoteData) { int result = 0; //if it doesn't match to any value on the dictionary it will return 0 Dictionary<int, int> quotes = new Dictionary<int, int(); quotes.add(7499, 5300); // not sure if that how you add values to a dictionary ... quotes.add(1049, 1000); for(Entry<int, int> element in quotes) //not sure about the enhanced for too hehehe, not using C# for a while { if(TotalChapter7(quoteData) >= element.key() && element.value > result) { result = element.value(); //don't break cause you have to test the entire list for acurracy } } return result; }