Is there a more elegant way to make this comparison? I cant imagine, check if received value is > or <.

range_1 = 1000

percent_1 = 10

range_2 = 1500

percent_2 = 15

range_3 = 1500.01

percent_3 = 20

class UtilsPurchase:
    @classmethod
    def calculate_cashback(cls, purchase_value):

        if purchase_value <= range_1:
            result = D(purchase_value / 100) * D(percent_1)
            return {'percent': percent_1, 'calculated_value': D(result)}

        if range_1 < purchase_value <= range_2:
            result = D(purchase_value / 100) * D(percent_2)
            return {'percent': percent_2, 'calculated_value': D(result)}

        if purchase_value >= range_3:
            result = D(purchase_value / 100) * D(percent_3)
            return {'percent': percent_3, 'calculated_value': D(result)}

CodePudding user response：

I would suggest using if, elif, and else statements to combine the three separate if statements into one conditional statement.

For instance:

if purchase_value <= range_1:
    result = D(purchase_value / 100) * D(percent_1)
    return {'percent': percent_1, 'calculated_value': D(result)}

elif purchase_value <= range_2:
    result = D(purchase_value / 100) * D(percent_2)
    return {'percent': percent_2, 'calculated_value': D(result)}

else:
    result = D(purchase_value / 100) * D(percent_3)
    return {'percent': percent_3, 'calculated_value': D(result)}

This would also solve the issue of your code not being able to perform calculations on a purchase_value that is between 1500 and 1500.01 (with the above code it would go into the else section).

CodePudding user response：

You can simplify your code by putting everything you repeat into a function:

class UtilsPurchase:
    @classmethod
    def calculate_cashback(cls, purchase_value):
        if purchase_value <= range_1:
            return cashback_helper(purchase_value, percent_1)

        elif purchase_value <= range_2:
            return cashback_helper(purchase_value, percent_2)

        elif purchase_value >= range_3:
            return cashback_helper(purchase_value, percent_3)


    def cashback_helper(purchase_value, percent):
        result = D(purchase_value / 100) * D(percent)
        return {'percent': percent, 'calculated_value': D(result)}

CodePudding user response：

This is pretty close to a text-book use case for the bisect module from the standard Python library. See the first example in the documentation. You can give the the breakpoints, the purchase_values and a particular value, it will tell what index you need, which you can use to find the percent you are interested in.

You haven't told us what D() does so I'm just using an identity function:

from bisect import bisect 

def D(v):
    return v

class UtilsPurchase:
    ranges = [1000, 1500]
    percents = [10, 15, 20]

    @classmethod
    def calculate_cashback(cls, purchase_value):
        percent = percents[bisect(ranges, purchase_value)]
        result = D(purchase_value / 100) * D(percent)
        return {'percent': percent, 'calculated_value': D(result)}

UtilsPurchase.calculate_cashback(1100)
# {'percent': 15, 'calculated_value': 165.0}

UtilsPurchase.calculate_cashback(5)
# {'percent': 10, 'calculated_value': 0.5}

This makes it easy to add more breakpoints without strings of if/elif blocks.

CodePudding user response：

This may reduce lines of code and avoid extra checking :

def GetFormattedResult(percent, purchase_value):
    result = D(purchase_value / 100) * D(percent)
    return {'percent': percent, 'calculated_value': D(result)}

class UtilsPurchase:
    @classmethod
    def calculate_cashback(cls, purchase_value):

        if purchase_value <= range_1:
            GetFormattedResult(purchase_value, range_1)

        elif range_1 < purchase_value <= range_2:
            GetFormattedResult(purchase_value, range_2)

        else: purchase_value >= range_3:
            GetFormattedResult(purchase_value, range_3)

CodePudding user response：

It's always best to minimize duplicated code. You can see all 3 of your if statements are almost entirely the same.

cutoffs = [1000, 1500, float("inf")]
percentages = [10, 15, 20]

for cutoff, percentage in zip(cutoffs, percentages):
    if purchase_value <= cutoff:
        result = D(purchase_value / 100) * D(percentage)
        return {'percent': percentage, 'calculated_value': D(result)}

CodePudding user response：

You could try something like this:

def calculate_cashback(purchase_value):
    values=[1000,1500]
    cashback=[0.10,0.15,20]
    c = 0
    for(i,t in enumerate(values)):
        if purchase_value > t:
            c = i 1
        else
            break
    
    return {'percent':cashback[c]*100,'calculated_value':purchase_value*(1-cashback[c]) }

Note, 'percent' will be return as a full integer and not a decimal. You can change it according to your needs.