Gfrktyl

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.

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

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.