# Cake-ordering form and calculator

I’m a designer learning to code and I’ve built a simple form calculator for ordering cakes.

I would love it if you could show me how to make the code more idiomatic. How can it be more consistent, concise and simplified? How can I align it with best practices? What’s the right way to group and order it? In what other ways can I make it better?

It doesn’t need to support anything older than ECMAScript 6.

// Cake-ordering calculator

const calculator = document.forms[0]
const total = calculator.elements.total

const occasionOptions = {
party: 20,
birthday: 25,
anniversary: 50,
wedding: 100
}

const sizeOptions = {
six: 1,
eight: 1.5,
ten: 2,
twelve: 2.5
}

const extrasOptions = {
inscription: 10,
decoration: 25,
special: 50
}

function cake() {
const cakes = Array.from(calculator.elements["cake"]).slice(0, 3)
const raphael = calculator.elements.raphael

function isChecked(checkbox) {
return checkbox.checked
}

let count = cakes.filter(isChecked).length
if (count) {
count = count * 0.5 + 0.5
}
if (raphael.checked) {
count += 1
}
return count
}

function occasion() {
let occasionCost = 0
const occasion = calculator.elements.occasion

for (let i = 0; i < occasion.length; i++) {
if (occasion[i].checked) {
occasionCost = occasionOptions[occasion[i].id]
break
}
}
return occasionCost
}

function size() {
let sizeIndex = 1
const size = calculator.elements.size

for (let i = 0; i < size.length; i++) {
if (size[i].checked) {
sizeIndex = sizeOptions[size[i].id]
break
}
}
return sizeIndex
}

function extras() {
let extrasCost = 0
const extras = calculator.elements.extras

for (let i = 0; i < extras.length; i++) {
if (extras[i].checked) {
extrasCost = extrasCost + extrasOptions[extras[i].id]
}
}
return extrasCost
}

function calculateTotal() {
let totalCost = cake() * occasion() * size() + extras()
total.value = "$" + totalCost.toLocaleString("en") } // Display "extras" fieldset when "wedding" occasion is selected const occasions = Array.from(calculator.elements.occasion) const fieldset = Array.from(document.getElementsByTagName("fieldset")) fieldset[3].style.display = "none" occasions.forEach(occasion => { occasion.addEventListener("click", () => { if (occasion.id == "wedding") { fieldset[3].style.setProperty("display", "inherit") } else { fieldset[3].style.setProperty("display", "none") } }) }) // Display cost after size has been selected const sizes = calculator.elements.size total.style.display = "none" for (let i = 0; i < sizes.length; i++) { sizes[i].onclick = function() { if (this.checked) { total.style.setProperty("display", "inherit") } else { total.style.setProperty("display", "none") } } } // Disable all fieldsets except the first one const disabledFieldsets = document.querySelectorAll( "fieldset:not(:first-of-type)" ) for (let i = 0; i < disabledFieldsets.length; i++) { disabledFieldsets[i].disabled = true } // Enable fieldsets sequentially on selection document.querySelectorAll("fieldset").forEach(fieldset => { fieldset.addEventListener("change", function() { let nextFieldset = this.nextElementSibling while (nextFieldset && !nextFieldset.disabled) { nextFieldset = nextFieldset.nextElementSibling } if (nextFieldset) { nextFieldset.disabled = false } }) }) // Reset form after all inputs in the first fieldset are deselected const cakeOptions = document.querySelectorAll( "fieldset:first-of-type input[type=checkbox]" ) let isChecked = false cakeOptions.forEach(function(resetWhenAllUnchecked) { resetWhenAllUnchecked.addEventListener("click", function(e) { if (this.checked) { isChecked = true } else { if ( isChecked && !document.querySelectorAll( "fieldset:first-of-type input[type=checkbox]:checked" ).length ) { calculator.reset() fieldset[3].style.setProperty("display", "none") total.style.setProperty("display", "none") } } }) }) <form> <fieldset> <legend>Select Cakes</legend> <label><input type="checkbox" name="cake" id="leonardo">Leonardo</label> <label><input type="checkbox" name="cake" id="donatello">Donatello</label> <label><input type="checkbox" name="cake" id="michelangelo">Michelangelo</label> <label><input type="checkbox" name="cake" id="raphael">Raphael</label> <p>If you select more than one cake, the other cakes are discounted 50%.</p> <p><small>Does not apply to Raphael.</small></p> </fieldset> <fieldset> <legend>Choose Occasion</legend> <label><input type="radio" name="occasion" id="party" required>Party</label> <label><input type="radio" name="occasion" id="birthday">Birthday</label> <label><input type="radio" name="occasion" id="anniversary">Anniversary</label> <label><input type="radio" name="occasion" id="wedding">Wedding</label> </fieldset> <fieldset> <legend>Choose Size</legend> <label><input type="radio" name="size" id="six" required>6-inch</label> <label><input type="radio" name="size" id="eight">8-inch</label> <label><input type="radio" name="size" id="ten">10-inch</label> <label><input type="radio" name="size" id="twelve">12-inch</label> </fieldset> <fieldset> <legend>Select Extras</legend> <label><input type="checkbox" name="extras" id="inscription">Inscription</label> <label><input type="checkbox" name="extras" id="decoration">Decoration</label> <label><input type="checkbox" name="extras" id="special">Special Frosting & Icing</label> </fieldset> <input type="text" name="total" readonly> <input type="submit" value="Submit"> </form> ## 2 Answers ## Feedback I see originally the code utilized document.querySelector("form"); to get the form element, but has since been changed to utilize document.forms[0]. I had planned to mention that using that property can be quicker (since a function doesn't need to be called) but now I don't have to. ## Suggestions ### Spread operator Because is utilized, the spread syntax can be used to add the HTML elements. For example, instead of using Array.from(): const cakes = Array.from(calculator.elements["cake"]).slice(0, 3)  Create an array and use the spread operator to put the elements into an array: const cakes = [...calculator.elements["cake"]].slice(0, 3);  The same is also true for occasions and fieldsets. ### Arrow functions Another feature that can be used to simplify things is arrow functions, which is actually used in the click handler for the occasion options. For example, the function isChecked: function isChecked(checkbox) { return checkbox.checked }  Can be simplified to an arrow function: const isChecked = checkbox => checkbox.checked;  This could actually be done to all functions if so desired, but beware that hoisting won't apply to arrow functions declared as a function expression. ### Wait for DOM to be ready I thought I mentioned this in my answer to your previous post but realize now that I hadn't (so I had to update my answer): it is best to wait for the DOM to be ready before performing DOM queries. One way to do this is to use document.addEventListener to add a callback function for the DOMContentLoaded event: document.addEventListener("DOMContentLoaded", function(event) { //code to execute now that the DOM is ready });  ### Cache DOM References As I mentioned in my answer to your previous post, DOM references can be stored in variables once to avoid repeated lookups. In the code here, cakes and raphael are declared within the cakes function, so every time that function runs it queries the DOM for those elements. ### Function names The function cake might be better named something like getCakeCount or getCakeMultiplier, since it is returning a number. ### Variable names The forEach iterator at the end has a currentValue argument named resetWhenAllUnchecked : cakeOptions.forEach(function(resetWhenAllUnchecked) {  resetWhenAllUnchecked sounds like a boolean value. A more appropriate name would be cakeOption. ### Early returns from functions The function occasion could be simplified from: function occasion() { let occasionCost = 0 const occasion = calculator.elements.occasion for (let i = 0; i < occasion.length; i++) { if (occasion[i].checked) { occasionCost = occasionOptions[occasion[i].id] break } } return occasionCost }  Instead of creating occasionCost, updating it if an option is checked and then returning that value, one could return as soon as a found item is checked. That way there is no need to break out of the for loop nor update the return value. Some may argue that it is better to have a single return statement at the end of the function but that is a preference/convention that must be agreed upon. function occasion() { const occasion = calculator.elements.occasion for (let i = 0; i < occasion.length; i++) { if (occasion[i].checked) { return occasionOptions[occasion[i].id]; } } return 0; }  ## Rewrite See code below using advice from suggestions above. I feel there are still some simplifications that could be done to the functions toward the end - perhaps that can be an exercise to the reader to do. document.addEventListener('DOMContentLoaded', _ => { //DOM references const calculator = document.forms[0] const total = calculator.elements.total const firstThreeCakeOptions = [...calculator.elements["cake"]].slice(0, 3) const raphael = calculator.elements.raphael const cakeOptions = document.querySelectorAll( "fieldset:first-of-type input[type=checkbox]" ) const occasions = [...calculator.elements.occasion] const fieldset = [...document.getElementsByTagName("fieldset")] const occasionOptions = { party: 20, birthday: 25, anniversary: 50, wedding: 100 } const sizeOptions = { six: 1, eight: 1.5, ten: 2, twelve: 2.5 } const extrasOptions = { inscription: 10, decoration: 25, special: 50 } calculator.addEventListener("click", calculateTotal) function cake() { const isChecked = checkbox => checkbox.checked; let count = firstThreeCakeOptions.filter(isChecked).length if (count) { count = count * 0.5 + 0.5 } if (raphael.checked) { count += 1 } return count } function occasion() { const occasion = calculator.elements.occasion for (let i = 0; i < occasion.length; i++) { if (occasion[i].checked) { return occasionOptions[occasion[i].id] } } return 0 } function size() { const size = calculator.elements.size for (let i = 0; i < size.length; i++) { if (size[i].checked) { return sizeOptions[size[i].id] } } return 1 } function extras() { let extrasCost = 0 const extras = calculator.elements.extras for (let i = 0; i < extras.length; i++) { if (extras[i].checked) { extrasCost = extrasCost + extrasOptions[extras[i].id] } } return extrasCost } function calculateTotal() { const totalCost = cake() * occasion() * size() + extras() total.value = "$" + totalCost.toLocaleString("en")
}

// Display "extras" fieldset when "wedding" occasion is selected

fieldset[3].style.display = "none"

occasions.forEach(occasion => {
if (occasion.id == "wedding") {
fieldset[3].style.setProperty("display", "inherit")
} else {
fieldset[3].style.setProperty("display", "none")
}
})
})

// Display cost after size has been selected

const sizes = calculator.elements.size

total.style.display = "none"

for (let i = 0; i < sizes.length; i++) {
sizes[i].onclick = function() {
if (this.checked) {
total.style.setProperty("display", "inherit")
} else {
total.style.setProperty("display", "none")
}
}
}

// Disable all fieldsets except the first one

const disabledFieldsets = document.querySelectorAll(
"fieldset:not(:first-of-type)"
)
for (let i = 0; i < disabledFieldsets.length; i++) {
disabledFieldsets[i].disabled = true
}

// Enable fieldsets sequentially on selection

document.querySelectorAll("fieldset").forEach(fieldset => {
let nextFieldset = this.nextElementSibling

while (nextFieldset && !nextFieldset.disabled) {
nextFieldset = nextFieldset.nextElementSibling
}

if (nextFieldset) {
nextFieldset.disabled = false
}
})
})

// Reset form after all inputs in the first fieldset are deselected

let isChecked = false

cakeOptions.forEach(function(resetWhenAllUnchecked) {
if (this.checked) {
isChecked = true
} else {
if (
isChecked &&
!document.querySelectorAll(
"fieldset:first-of-type input[type=checkbox]:checked"
).length
) {
calculator.reset()
fieldset[3].style.setProperty("display", "none")
total.style.setProperty("display", "none")
}
}
})
})
});
<form>
<fieldset>
<legend>Select Cakes</legend>
<label><input type="checkbox" name="cake" id="leonardo">Leonardo</label>
<label><input type="checkbox" name="cake" id="donatello">Donatello</label>
<label><input type="checkbox" name="cake" id="michelangelo">Michelangelo</label>
<label><input type="checkbox" name="cake" id="raphael">Raphael</label>
<p>If you select more than one cake, the other cakes are discounted 50%.</p>
<p><small>Does not apply to Raphael.</small></p>
</fieldset>

<fieldset>
<legend>Choose Occasion</legend>
</fieldset>

<fieldset>
<legend>Choose Size</legend>
</fieldset>

<fieldset>
<legend>Select Extras</legend>
<label><input type="checkbox" name="extras" id="inscription">Inscription</label>
<label><input type="checkbox" name="extras" id="decoration">Decoration</label>
<label><input type="checkbox" name="extras" id="special">Special Frosting & Icing</label>
</fieldset>

<input type="submit" value="Submit">
</form>

• Hey Sam, I found a problem. If you choose "wedding", then select any of the "extras", and afterwords choose "party", "birthday", or "anniversary", "extras" are still added to the total cost instead of being subtracted. I know how to uncheck the "extras" checkboxes on "wedding" deselection – extras.forEach(extra => extra.checked = false) – but I would like to remember their checked state in case "wedding" is selected again.
– Tzar
Aug 24, 2018 at 7:49
• My initial thought is to say one option is to return 0 in the extras() function whenever the occasion is not wedding, but then after considering the Separation of Concerns principle I wonder if that logic should be put into calculateTotal()... Aug 25, 2018 at 13:12

Here is the rewritten code with @SamOnela’s suggestions applied, and many other little modifications.

document.addEventListener("DOMContentLoaded", _ => {
//DOM references

const calculator = document.forms[0]
const total = calculator.elements.total
const firstThreeCakeOptions = [...calculator.elements.cake].slice(0, 3)
const size = calculator.elements.size
const occasions = [...calculator.elements.occasion]
const fieldset = [...document.getElementsByTagName("fieldset")]
const extrasFieldset = fieldset[3]

// Prices

const occasionOptions = {
party: 20,
birthday: 25,
anniversary: 50,
wedding: 100
}

const sizeOptions = {
six: 1,
eight: 1.5,
ten: 2,
twelve: 2.5
}

const extrasOptions = {
inscription: 10,
decoration: 25,
special: 50
}

// Calculator logic

function cakeMultiplier() {
const size = calculator.elements.size
const isChecked = checkbox => checkbox.checked

let count = firstThreeCakeOptions.filter(isChecked).length
if (count) {
count = count * 0.5 + 0.5
}
if (raphael.checked) {
count += 1
}
return count
}

function occasionCost() {
const occasion = calculator.elements.occasion

for (let i = 0; i < occasion.length; i++) {
if (occasion[i].checked) {
return occasionOptions[occasion[i].id]
}
}
return 0
}

function sizeCost() {
for (let i = 0; i < size.length; i++) {
if (size[i].checked) {
return sizeOptions[size[i].id]
}
}
return 1
}

function extrasCost() {
const extras = calculator.elements.extras
let extrasCost = 0

for (let i = 0; i < extras.length; i++) {
if (extras[i].checked) {
extrasCost = extrasCost + extrasOptions[extras[i].id]
}
}
return extrasCost
}

function calculateTotal() {
const totalCost =
cakeMultiplier() * occasionCost() * sizeCost() + extrasCost()
total.value = "\$" + totalCost.toLocaleString("en")
}

// Display "extras" fieldset when "wedding" occasion is selected

extrasFieldset.style.display = "none"

occasions.forEach(occasion => {
if (occasion.id == "wedding") {
extrasFieldset.style.setProperty("display", "inherit")
} else {
extrasFieldset.style.setProperty("display", "none")
}
})
})

// Display cost after size has been selected

total.style.display = "none"

size.forEach(i => {
if (this.checked) {
total.style.setProperty("display", "inherit")
}
})
})

// Disable all fieldsets except the first one

function disableFieldsets() {
const disabledFieldsets = document.querySelectorAll(
"fieldset:not(:first-of-type)"
)

for (let i = 0; i < disabledFieldsets.length; i++) {
disabledFieldsets[i].disabled = true
}
}

disableFieldsets()

// Enable fieldsets sequentially on selection

document.querySelectorAll("fieldset").forEach(fieldset => {
let nextFieldset = this.nextElementSibling

while (nextFieldset && !nextFieldset.disabled) {
nextFieldset = nextFieldset.nextElementSibling
}

if (nextFieldset) {
nextFieldset.disabled = false
}
})
})

// Reset form after all inputs in the first fieldset are deselected

let isChecked = false

document
.querySelectorAll("fieldset:first-of-type input[type=checkbox]")
.forEach(function(cakeOption) {
if (this.checked) {
isChecked = true
} else {
if (
isChecked &&
!document.querySelectorAll(
"fieldset:first-of-type input[type=checkbox]:checked"
).length
) {
calculator.reset()
disableFieldsets()
extrasFieldset.style.setProperty("display", "none")
total.style.setProperty("display", "none")
}
}
})
})
})
<form>
<fieldset>
<legend>Select Cakes</legend>
<input type=checkbox name=cake id=leonardo>
<label for=leonardo>Leonardo</label>

<input type=checkbox name=cake id=donatello>
<label for=donatello>Donatello</label>

<input type=checkbox name=cake id=michelangelo>
<label for=michelangelo>Michelangelo</label>

<input type=checkbox name=cake id=raphael>
<label for=raphael>Raphael</label>

<p>If you select more than one cake, the other cakes are discounted 50%.
<p><small>Does not apply to Raphael.</small>
</fieldset>

<fieldset>
<legend>Choose Occasion</legend>
<label for=party>Party</label>

<label for=birthday>Birthday</label>

<label for=anniversary>Anniversary</label>

<label for=wedding>Wedding</label>
</fieldset>

<fieldset>
<legend>Choose Size</legend>
<label for=six>6-inch</label>

<label for=eight>8-inch</label>

<label for=ten>10-inch</label>

<label for=twelve>12-inch</label>
</fieldset>

<fieldset>
<legend>Select Extras</legend>
<input type=checkbox name=extras id=inscription>
<label for=inscription>Inscription</label>

<input type=checkbox name=extras id=decoration>
<label for=decoration>Decoration</label>

<input type=checkbox name=extras id=special>
<label for=special>Special Frosting & Icing</label>
</fieldset>

</form>