Tip Calculator in pure JS

Question

A simple tip calculator that asks for the total bill amount, service quality (used to determine what percentage of the total cost will be tipped), and how many people are sharing the bill (used to split the tip so that everyone pays the same amount).

I am primarily looking for possible improvements to performance or readability, but any suggestion is welcome.

See the code in action - jsFiddle

Link to repository - GitHub

// Pointers
const billAmountAlertPointer = document.getElementById('billAmountAlert');
const tipPercentageAlertPointer = document.getElementById('tipPercentageAlert');
const numberOfPeoplePointer = document.getElementById('numberOfPeople');
const serviceQualityPointer = document.getElementById('serviceQuality');
const numberOfPeopleLabelPointer = document.getElementById('numberOfPeopleLabel');
const eachPointer = document.getElementById('each');
const soundPointer = document.getElementById('cashRegisterSound');
soundPointer.volume = 0.05;
var numberOfPeople = 1;

// Display the text 'people' next to the input if the number of people is higher than 1
numberOfPeoplePointer.oninput = function(){
    numberOfPeople = numberOfPeoplePointer.value;
    numberOfPeople > 1 ? numberOfPeopleLabelPointer.innerHTML = 'people' : numberOfPeopleLabelPointer.innerHTML = 'lone wolf';
};

document.getElementById('calculateButton').addEventListener('click', function() {
    let billAmount = document.getElementById('billAmount').value;
    let tipPercentage = serviceQualityPointer.value;
    // Only proceed if both the billAmount and the tipPercentage have a value
    if (billAmount !== '' && tipPercentage !== ''){
        setDisplay(billAmountAlertPointer, 'none');
        setDisplay(tipPercentageAlertPointer, 'none');   
        let tipAmount = (billAmount / (100 / tipPercentage)) / numberOfPeople;
        document.getElementById('tipAmount').innerHTML = '$' + tipAmount;
        /* If the number of people sharing the bill is higher than 1, display the text 'each'
        next to the input. Otherwise, don't.*/
        numberOfPeople > 1 ? setDisplay(eachPointer, 'block') : setDisplay(eachPointer, 'none');
        document.getElementById('tipAmountContainer').style.display = 'block';

        // Play the cash register sound
        playSound();
    } else{
        // If the user hasn't entered the bill amount, display an alert
        if (billAmount === ''){
            setDisplay(billAmountAlertPointer, 'inline');
        }
        // If the user hasn't entered the tip percentage, display an alert
        if (tipPercentage === ''){
            setDisplay(tipPercentageAlertPointer, 'inline');
        }
    }
});

function setDisplay(element, value){
    element.style.display = value;
}

function playSound(){
    soundPointer.play();
}

Answer 1

Review

For what it does that is very dense code. Consider reducing variable name length.

Major points

• Direct element reference reduces code size, improves performance, and forces you to ensure element IDs and names are unique.
• Adding content via innerHTML, clobbers events, forces reflows, and chews power.
• Incorrect currency handling. Money comes in cents, not fractions of a cent. Always use integer math for calculations that involve cash exchanges.

Minor points

• Avoid adding event listeners directly to the element's on property. Use addEventListener
• There are some cases where innerHTML can be useful, most of the time it is the worst way to add to the DON. Don't use it!
• Add text content via element.textContent.
• Don't change element style properties directly, create CSS rules and set the appropriate class name.
• Use const for constants.
• Don't add comments that point out the obvious.
• Use arrow functions for anon functions;

Direct element reference

All elements that have an Id property set can be accessed by that id directly in the global namespace.

<div id="divEl">Foo</div>
<script>
    const element = document.getElementById("divEl");
    // is the same as
    const element = divEl;

    ...

This works on all browsers (and has done since netscape died). You MUST ensure that id's are unique, if not your page can not be validated and will enter quirks mode. Browser behaviour will differ if in quirks mode.

Using direct referencing forces you to ensure id's are unique and thus avoid a very common cause of quirky DOM modes. It also greatly reduces the overhead of needing to use DOM queries to locate elements.

Direct referenced elements are live.

Working with money

When working with currencies you need to remember that money is exchanged in integer units. For the US that is cents. Your calculations do not handle the tip correctly. eg bill = $1000, tip 10%, for 3, means each person must pay $33.33333...

Always work in cents, always round to cent, or round up if there is an exchange priority. In this case you convert the bill to cents, calculate the tip per person, rounding up to nearest cent then convert back to dollars.

bill *= 100;                        // convert to cents
bill = Math.round(bill);            // round to nearest
tip = bill / 100 * tip / people;    // get tip in cents per person
tip = Math.ceil(tip);               // round up
total = bill + tip * people;        // Get total in cents         

When displaying currency always use Number.toLocaleString. Eg displaying the above values.

tipPerPersonDisplay = (tip / 100).toLocaleString("en-US", {style: "currency", currency: "USD"});
totalBillDisplay = (total / 100).toLocaleString("en-US", {style: "currency", currency: "USD"});

Example

Note that element ids have been prepended with El as I do not know what other code or markup you have.

The code size has be reduce by half meaning its is easier to maintain and read.

Money is handled correctly and rounded up to favour the tip (a cent per person max)

const CURRENCY = ["en-US", {style: "currency", currency: "USD"}];
const setElClass = (el, cName, show = true) => el.classList[show ? "add" : "remove"](cName);

numberOfPeopleEl.addEventListener("input", () => {
    numberOfPeopleLabelEl.textContent = numberOfPeopleEl.value > 1 ? "people" : "lone wolf";
});

calculateButtonEl.addEventListener('click', () => {
    var tip = serviceQualityEl.value;
    const bill = billAmountEl.value;
    const people = numberOfPeopleEl.value;

    setElClass(billAmountAlertEl, "alert--show", isNaN(bill));
    setElClass(tipPercentageAlertEl, "alert--show", isNaN(tip)); 

    if (!isNaN(bill) && !isNaN(tip)) {
        tip = Math.ceil(Math.round(bill * 100) / 100 * tip / people);  // in cents per person
        tipAmountEl.textContent = (tip / 100).toLocaleString(...CURRENCY);
        setElClass(eachEl, "people-count--show", people > 1);
        tipAmountContainerEl.classList.add("tip-amounts--show");
        cashRegisterSoundEl.volume = 0.05;
        cashRegisterSoundEl.play();
    }
});

You will also need the following CSS rules.

.alert--show { display : inline }
.people-count--show { display : inline }
.tip-amounts--show { display : block }