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>

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>