Calculating tips

Question

I am enrolled in an online JavaScript course and one of our coding challenges was presented as follows:

John and his family went on a holiday and went to 3 different restaurants. The bills were 124,ドル 48,ドル and 268ドル.

To tip the waiter a fair amount, John created a simple tip calculator (as a function). He likes to tip 20% of the bill when the bill is less than 50,ドル 15% when the bill is between 50ドル and 200,ドル and 10% if the bill is more than 200ドル.

In the end, John would like to have 2 arrays:

1. Containing all three tips (one for each bill)
2. Containing all three final paid amounts (bill + tip)

I have come up with the following solution:

var bills = [124, 48, 268];
var totals = [];
var pointlessArray = [];
function calculateTip(cost) {
	switch (true) {
		case cost < 50:
			return cost * .2;
			break;
		case cost > 49 && cost < 201:
			return cost * .15;
			break;
		case cost > 200:
			return cost * .1;
			break;
		default:
			Error('Unsupported input.');
	}
}
function makePointlessArray(inputArray) {
	var length = inputArray.length;
	for (var i = 0; i < length; i++) {
		pointlessArray[i] = calculateTip(inputArray[i]);
	}
}
function calculateTotal(billArray) {
	var length = billArray.length;
	for (var i = 0; i < length; i++) {
		totals[i] = billArray[i] + calculateTip(billArray[i]);
	}
}
makePointlessArray(bills);
calculateTotal(bills);
console.log(`The bills are: ${bills}`);
console.log(`The calculated tips are: ${pointlessArray}`);
console.log(`The calculated totals are: ${totals}`);

I don't think this is practical at all for calculating tips, but have tried to stay within the parameters of the challenge.

I am unsure if declaring the arrays as global variables is the best practice or if some other method should be used, but as a JS newbie I would appreciate any input on pitfalls in my code.

• Cris Luengo: So the waiter gets more money if the bill is 49ドル than if it’s 51ドル? Weird... :)

Answer 1

break statements after return

While it is a great habit to include the break statements, there is no need to include break statements following a return, since anything following the return statement within a function is unreachable.

switch(true)

while this works because each case statement must evaluate to true, it is simpler to just use if statements. Your logic checks if the value is less than 50, then if it is greater than 49 and less than 201. The requirements never specified if the values would be integers, floats, etc. So it would be wise to consider a value like 49.5. That value would be less than 50 so the first condition would be true. However if the value was 200ドル.50 the second condition would evaluate to true, even though the value was more than 200. So update the condition to cost <= 200. Otherwise if neither of those conditions have been met, the value must be more than 200.

function calculateTip(cost) {
 if (cost < 50) {
 return cost * .2;
 }
 if (cost <= 200) {
 return cost * .15;
 }
 return cost * .1;
}

Error

The default case of the switch statement merely calls Error

default:
 Error('Unsupported input.');

This likely won't do what you expect. If you want an error to be thrown, then instantiate an error with the new operator and precede it with the throw operator:

throw new Error('Unsupported input.');

It would be wise the check the input before comparing the value against other numbers, perhaps with the parseFloat() function:

function calculateTip(cost) {
 if (!parseFloat(cost)) {
 throw new Error('Unsupported input.');
 }

That way anything that cannot be coerced the an integer (e.g. {}) will cause the error to be thrown.

Updated code

See the code below with the advice above implemented.

var bills = [124, 48, 268];
var totals = [];
var pointlessArray = [];
function calculateTip(cost) {
 if (!parseFloat(cost)) {
 throw new Error('Unsupported input.');
 }
 if (cost < 50) {
 return cost * .2;
 }
 if (cost <= 200) {
 return cost * .15;
 }
 return cost * .1;
}
function makePointlessArray(inputArray) {
 var length = inputArray.length;
 for (var i = 0; i < length; i++) {
 pointlessArray[i] = calculateTip(inputArray[i]);
 }
}
function calculateTotal(billArray) {
 var length = billArray.length;
 for (var i = 0; i < length; i++) {
 totals[i] = billArray[i] + calculateTip(billArray[i]);
 }
}
makePointlessArray(bills);
calculateTotal(bills);
console.log(`The bills are: ${bills}`);
console.log(`The calculated tips are: ${pointlessArray}`);
console.log(`The calculated totals are: ${totals}`);

• Blindman67: You forgot that cash money does not exchange in fractions of a cent. Doubles and cash do not mix well.... The OP's questions question is a classic "don't forget to round" trap.

Answer 2

There's a small bug in your code: a bill of 200ドル.50 will be tipped at the wrong amount since you are testing for cost < 201.

It's probably easier to just us ifs that are sorted and return the tip from the function on the first one that passes. That avoids the need to test ranges like cost > 49 && cost < 201. If there were lots of categories you could make a lookup table that would work like this:

const tiptable = [
 {min: 200, tip: .1},
 {min: 50, tip: .15},
 {min: 0, tip: .2} 
]
let cost = 51
let tip = tiptable.find(tip => cost > tip.min).tip
console.log(tip)
`find()` will give you the first object that matches. But that only makes sense if there are enough categories that `if` statement get unwieldy.

find() will give you the first object that matches. But that only makes sense if there are enough categories that if statement get unwieldy.

Other parts of the code can be simplified with some of the nice functional tools javascript gives us. You can make a one-line sum using reduce() and you can make an array of tips using map() allowing you to make the arrays directly without for loops.

For example:

const bills = [124, 48, 268];
function getTip(cost){
 if (cost < 50) return .2
 if (cost <= 200) return .15 // not clear whether this should be < or <=
 return .1
}
let with_tip = bills.map(cost => cost + cost * getTip(cost) )
let sum = with_tip.reduce((total, n) => total + n)
console.log('Bills with tip: ', with_tip.map(t => t.toLocaleString("en-US", {style: 'currency', currency: 'USD'})))
console.log('Total:', sum.toLocaleString("en-US", {style: 'currency', currency: 'USD'}))

Also, just as an example I added nicely-formatted currency strings with toLocaleString.

• Blindman67: Great answer +1 Would give another +1 for the tiptable if I could (BTW should be tipTable) and +1 for toLocaleString. One tiny problem. What if there are 6 bills of 6ドル.06 each. Your code reports the total as 43ドル.63 which is not a factor of 6, who gets the extra cent? The correct answer is 43ドル.62 with each tip being the same amount. Money values are discrete and you have forgotten to round to nearest cent each time you do a calculation. Always work in whole cents rounding each calculation or you will end up trying to split fractions of a cent.
• Mark M: Yes, that a great point @Blindman67 and I totally agree about working with whole cents.
• Sᴀᴍ Onᴇᴌᴀ: It would be wise to disclose that ecmascript-6 features like arrow functions and let/const have certain browser compatibilities to be aware of- while most modern browsers support them, certain users might be using legacy browsers