Cake-ordering form and calculator

Question

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
}
calculator.addEventListener("click", calculateTotal)
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>

Answer 1

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 ecmascript-6 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 ecmascript-6 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 => {
 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
 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>

• Tzar: 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.
• Sᴀᴍ Onᴇᴌᴀ: 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()...

Answer 2

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
 calculator.addEventListener("click", calculateTotal)
 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 => {
 occasion.addEventListener("click", () => {
 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 => {
 i.addEventListener("click", function(e) {
 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 => {
 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
 let isChecked = false
 document
 .querySelectorAll("fieldset:first-of-type input[type=checkbox]")
 .forEach(function(cakeOption) {
 cakeOption.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()
 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>
 <input type=radio name=occasion id=party required>
 <label for=party>Party</label>
 
 <input type=radio name=occasion id=birthday>
 <label for=birthday>Birthday</label>
 
 <input type=radio name=occasion id=anniversary>
 <label for=anniversary>Anniversary</label>
 
 <input type=radio name=occasion id=wedding>
 <label for=wedding>Wedding</label>
</fieldset>
<fieldset>
<legend>Choose Size</legend>
 <input type=radio name=size id=six required>
 <label for=six>6-inch</label>
 
 <input type=radio name=size id=eight>
 <label for=eight>8-inch</label>
 
 <input type=radio name=size id=ten>
 <label for=ten>10-inch</label>
 
 <input type=radio name=size id=twelve>
 <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>
<input type=text name=total readonly>
<input type=submit value=Submit>
</form>

• Vogel612: could you please elaborate a bit on the "many other little modifications"? It's a bit difficult to see what you did without a diffing tool. Thanks :)
• Tzar: @Vogel612 First and foremost, I applied Sam’s suggestions, then I tidied up the JavaScript code by reordering it and renaming a few things, and I also made the HTML a bit more concise and more readable.
• Tzar: The code snippet in my answer should be the starting point.