Calculating cost of mobile phone bill

[assembly21]

Hello:

I’m doing this exercise from a book:

Suppose your cell phone carrier charges you $29.95 for up to 300 miutes of calls, and $0.45 for each additional minute, plus 12.5 percent taxes and fees. Write a programme to compute the monthly charge from a given number of minutes.

This is what I wrote:

minutesIncluded = 300

costMonthlyPlan = 29.95

costPerMinute = 0.45;

taxes = 12.5

minutesUsed = int(input("How many minutes did you use this month?: "))

if minutesUsed < minutesIncluded :
    taxes = costMonthlyPlan * 12.5 / 100
    monthlyBill = costMonthlyPlan + taxes

else :
    difference = minutesUsed - minutesIncluded
    extraCost = difference * costPerMinute
    taxes = (costMonthlyPlan + extraCost) * 12.5 / 100
    monthlyBill = costMonthlyPlan + extraCost + taxes

print("Your monthly bill is: ", monthlyBill)

This works but I’d like to know if this this how you would do it. Thanks!

[megazoid]

I would remove code duplication on taxes calculation

if minutesUsed < minutesIncluded :
    monthlyBill = costMonthlyPlan
else :
    difference = minutesUsed - minutesIncluded
    extraCost = difference * costPerMinute
    monthlyBill = costMonthlyPlan + extraCost

taxes = monthlyBill * 12.5 / 100
total = monthlyBill + taxes

So if you decide to change 12.5 to some other value you will have to fix one line instead of two.
Maybe it doesn’t matter too much in this particular simple program, but it is a good practice to start using.

Edit: also I realised there is no need for else at all, whole logic can be shortened to single if:

monthlyBill = costMonthlyPlan

if minutesUsed > minutesIncluded :
    difference = minutesUsed - minutesIncluded
    extraCost = difference * costPerMinute
    monthlyBill += extraCost

taxes = monthlyBill * 12.5 / 100
total = monthlyBill + taxes

[assembly21]

and it’s good practice to have the if without the else, right?

[megazoid]

@assembly21

and it’s good practice to have the if without the else, right?

The main idea here is to keep your code as simple, as possible.
(and that is why I like that you used additional difference and extraCost variables instead of writing

taxes = (costMonthlyPlan + (minutesUsed - minutesIncluded)*costPerMinute) * 12.5 / 100

which would be very hard to understand)

Each else operator adds some complexity.
Of course that doesn’t mean you shouldn’t use else at all.
It always depends on particular case.
But I believe that in most cases any task can be solved without else .

For example, I often see code like this:

if (user_is_logged) {
    // ... many ...
    // ... lines ...
    // ... of code ...
    // ... here ...
} else {
    go_to_login();
}

When you look at this code in real life you don’t know about else until you scroll enough to see it.
Also whole logic is nested into condition. This decreases readability of the code.

Instead, It can be rewritten that way:

if (user_is_NOT_logged) { go_to_login(); return; }
// ... many ...
// ... lines ...
// ... of code ...
// ... here ...

Now main logic is indenpendent from any conditions but you still see all the requirements before logic (user should be logged and if he isn’t, sent him to login routine). So we collapsed large nested block into single line.

[Gandalf]

It would also be neat to round the total to 2 places.

EDIT: I also notice you set taxes but then use a constant for the tax rate.