income calculator

Question

I'm really new to code optimization and JS. And I like to learn with clean code. I would like to ask for help on optimizing my code.

The code below parses the value of four inputs as floating point numbers and adds them together. The sum is then displayed in an output element.

  if(document.getElementById('salary-field').value ===''){
    salary = 0;
   }else{
    salary = parseFloat(document.getElementById('salary-field').value).toFixed(2);
    document.getElementById('salary-field').value = salary;
   } 
    
   if(document.getElementById('commission-field').value ===''){
    commission = 0;
   }else{
    commission = parseFloat(document.getElementById('commission-field').value).toFixed(2);
    document.getElementById('commission-field').value = commission;
   } 
   
   if(document.getElementById('rental-field').value ===''){
    rental = 0;
   }else{
     rental = parseFloat(document.getElementById('rental-field').value).toFixed(2);
     document.getElementById('rental-field').value = rental;
   } 
    
   if(document.getElementById('other-income-field').value ===''){
    other_income = 0;
   } else {
     other_income = parseFloat(document.getElementById('other-income-field').value).toFixed(2);
     document.getElementById('other-income-field').value = other_income;
   }
  
    sum = parseFloat(salary) + parseFloat(commission) + parseFloat(rental) + parseFloat(other_income);
    document.getElementById('sum-field').value = sum.toFixed(2);

Answer 1

From a short review;

• You should declare your variables with const or let

• There is a ton of copy-pasted code in there

• 'commission-field' is not a great name for your element, I would just call it commission

• Avoid writing multiple calls to document.getElementById for the same id

• Its hard to control what users enter, and sometimes parseFloat returns a NaN, I would always deal with that pro-actively

• I would create a function that formats an element like

  function cleanAmountElement(id){
    const elementValue = document.getElementById(id);
    const cleanerValue = parseFloat(elementValue*1).toFixed(2);
    const cleanestValue = IsNaN(cleanerValue)?0:cleanerValue;    
    document.getElementById(id).value = cleanestValue;
    return cleanestValue;
  } 
  

• Note that this is not super super clean, that function both updates the UI and returns a value :/

• Then you can do something funky with functional programming where you pass a list of ids and process them

  const sum = ['salary', 'commission', 'rental', 'other_income'].map(cleanAmountElement)
                                                                .reduce(getSum,0);
  
  function getSum(a,b){
    return a+b;
  }