3
\$\begingroup\$

As I am still new to web development, refactoring my code is still something that gives me some trouble. I would really appreciate it if someone could look over the Javascript for an interest rate calculator I built, and maybe give me some pointers on how to refactor it/improve on its efficiency.

It consists of three functions:

The first, createOptions, just loops over the numbers 1 to 100, and appends each of these numbers as an option to the drop down menu for the percentage input.

The second, calculateInterest, is self-explanatory. The formula used assumes compound interest. It also contains a loop which iterates over each payment until the payment period entered is reached. By compound interest, I mean the percentage of the initial amount (the principal) representing the interest rate is calculated, added to the principal, and the percentage of their sum (the interest plus the principal) is added to that. This process repeats for each iteration of the loop.

The last function onClick gets the form data in the DOM, stores them in variables, then invokes calculateInterest, passing the variables into its parameters.

I am mainly interested in advice regarding my Javascript but any HTML and CSS pointers would be appreciated, too.

createOptions();

function createOptions(){
  let select = document.getElementById('percentage');
  for(var i = 0; i <= 100; i++){ 
  var node = document.createElement("option");
  node.innerHTML = i;
  node.setAttribute('value', i);
  node.setAttribute('class', 'percent');
  select.appendChild(node); 
  }
}


function calculateInterest(amount, payments, interest){
  var total = amount;
  for(var i = 1; i <= payments; i++){
    var percent = total * interest;
    total = total += percent;
  }
  return '$' + total.toFixed(2); 
}

function onClick(){

  var para = document.getElementById('show');
  var result = document.getElementsByTagName('div')

  var select = document.getElementById('percentage');
  var percentValue = select.options[select.selectedIndex].value / 100;
  var amounts = document.getElementById('amount');
  var amountValue = parseFloat(amounts.value);
  var time = document.getElementById('time');
  var timeValue = parseInt(time.value);

  if(para.className === "show test"){
    para.remove()
    var para = document.createElement('p');
    para.id = "show"
    var result = document.getElementById('result')
    result.appendChild(para);
  }

  para.innerHTML = calculateInterest(amountValue, timeValue, percentValue);
  para.className = "show";
  para.className += " test"

}
body {
  background: linear-gradient( rgb(25, 25, 230), rgb(255, 255, 255));
  background-repeat: no-repeat;
  box-sizing: border-box;

    }

    h1 {
      text-align: center;

    }

    #title {
      font-family: Verdana;
      text-transform: uppercase;
      color: white;
    }
    label {
      font-weight: bolder;
      color: black;
    }
    .btn, .btn-light {
      margin-top: 10px;

    }

    #result {
      margin-top: 10px;
      font-size: 24px;
      text-align: center;

    }

    .wrap {
      position: relative;
      left: 45%;
    }

    .show {
      animation: fadeIn 1.8s ease-in 0.2s 1 normal both running;
    }

    @keyframes fadeIn {
      from {
        opacity: 0; 
      }

      to {
        display: block;
        opacity: 1;
      }
    }

    .test {

      font-size: 60px;
    }
<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" integrity="sha384-MCw98/SFnGE8fJT3GXwEOngsV7Zt27NXFoaoApmYm81iuXoPkFOJwJ8ERdknLPMO" crossorigin="anonymous">
<link rel="stylesheet" type="text/css" href="./style.css">


  <h1 id = "title">Interest Rate Calculator</h1>
<div class="container">
 <div class = "wrap">
   <label>% INTEREST</label>
   <select class = 'form-control' style = "width: 10%" name="percentage" id= "percentage"></select>

   <label> $ AMOUNT</label>
   <input type="number" min = '1' max = '999999999' name = "amount" id = "amount" class = "form-control" style = "width: 10%">
   <label for=""># OF PAYMENTS</label>
   <input type = "number" min = '1' max = '999' id = "time" class = "form-control" style = "width: 10%">
    <button onClick = "onClick()" target = "_top" class = 'btn btn-light'>CALCULATE!</button>  
   </div>
 </div>


 <div id="result">
   <p id = "show"></p>
 </div>
  <script type="text/javascript" src = "./script.js"></script>

Improve this question
\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

Style

You should consistently indent your CSS. This yields the following:

body {
    background: linear-gradient(#1919e6 ,#fff);
    background-repeat: no-repeat;
    box-sizing: border-box
}

h1 {
    text-align: center
}

#title {
    font-family: Verdana;
    text-transform: uppercase;
    color: #fff
}

label {
    font-weight: bolder;
    color: #000
}

.btn,.btn-light {
    margin-top: 10px
}

#result {
    margin-top: 10px;
    font-size: 24px;
    text-align: center
}

.wrap {
    position: relative;
    left: 45%
}

.show {
    animation: fadeIn 1.8s ease-in .2s 1 normal both running
}

@keyframes fadeIn {
    0% {
        opacity: 0
    }

    to {
        display: block;
        opacity: 1
    }
}

.test {
    font-size: 60px
}

Naming

You should name the onClick function something more descriptive. In addition, the for loop should be indented.

createOptions();

function createOptions(){
  let select = document.getElementById('percentage');
  for(var i = 0; i <= 100; i++){
     var node = document.createElement("option");
     node.innerHTML = i;
     node.setAttribute('value', i);
     node.setAttribute('class', 'percent');
     select.appendChild(node);
  }
}


function calculateInterest(amount, payments, interest){
  var total = amount;
  for(var i = 1; i <= payments; i++){
    var percent = total * interest;
    total = total += percent;
  }
  return '$' + total.toFixed(2);
}

function handler(){

  var para = document.getElementById('show'),
      result = document.getElementsByTagName('div')

  var select = document.getElementById('percentage'),
      percentValue = select.options[select.selectedIndex].value / 100,
      amounts = document.getElementById('amount'),
      amountValue = parseFloat(amounts.value),
      time = document.getElementById('time'),
      timeValue = parseInt(time.value);

  if(para.className === "show test"){
    para.remove();
    para = document.createElement('p');
    para.id = "show";
    result = document.getElementById('result');
    result.appendChild(para);
  }

  para.innerHTML = calculateInterest(amountValue, timeValue, percentValue);
  para.className = "show";
  para.className += " test";
}

HTML

Indentation could be a little better, and a doctype should be declared:

<!DOCTYPE html>
<html>

<head>
    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" integrity="sha384-MCw98/SFnGE8fJT3GXwEOngsV7Zt27NXFoaoApmYm81iuXoPkFOJwJ8ERdknLPMO" crossorigin="anonymous">
    <link rel="stylesheet" type="text/css" href="./style.css">
</head>

<body>
    <h1 id="title">Interest Rate Calculator</h1>
    <div class="container">
        <div class="wrap">
            <label>% INTEREST</label>
            <select class="form-control" style="width: 10%" name="percentage" id="percentage"></select>

            <label> $ AMOUNT</label>
            <input type="number" min="1" max="999999999" name="amount" id="amount" class="form-control" style="width: 10%">
            <label for=""># OF PAYMENTS</label>
            <input type="number" min="1" max="999" id="time" class="form-control" style="width: 10%">
            <button onclick="handler()" target="_top" class="btn btn-light">CALCULATE!</button>
        </div>
    </div>

    <div id="result">
        <p id="show"></p>
    </div>
    <script type="text/javascript" src="./script.js"></script>
</body>

</html>

Rewrite:

createOptions();

function createOptions(){
  let select = document.getElementById('percentage');
  for(var i = 0; i <= 100; i++){
     var node = document.createElement("option");
     node.innerHTML = i;
     node.setAttribute('value', i);
     node.setAttribute('class', 'percent');
     select.appendChild(node);
  }
}


function calculateInterest(amount, payments, interest){
  var total = amount;
  for(var i = 1; i <= payments; i++){
    var percent = total * interest;
    total += percent;
  }
  return '$' + total.toFixed(2);
}

function handler(){

  var para = document.getElementById('show'),
      result = document.getElementsByTagName('div')

  var select = document.getElementById('percentage'),
      percentValue = select.options[select.selectedIndex].value / 100,
      amounts = document.getElementById('amount'),
      amountValue = parseFloat(amounts.value),
      time = document.getElementById('time'),
      timeValue = parseInt(time.value);

  if(para.className === "show test"){
    para.remove()
    para = document.createElement('p');
    para.id = "show"
    result = document.getElementById('result')
    result.appendChild(para);
  }

  para.innerHTML = calculateInterest(amountValue, timeValue, percentValue);
  para.className = "show";
  para.className += " test"

}
body {
    background: linear-gradient(#1919e6 ,#fff);
    background-repeat: no-repeat;
    box-sizing: border-box
}

h1 {
    text-align: center
}

#title {
    font-family: Verdana;
    text-transform: uppercase;
    color: #fff
}

label {
    font-weight: bolder;
    color: #000
}

.btn,.btn-light {
    margin-top: 10px
}

#result {
    margin-top: 10px;
    font-size: 24px;
    text-align: center
}

.wrap {
    position: relative;
    left: 45%
}

.show {
    animation: fadeIn 1.8s ease-in .2s 1 normal both running
}

@keyframes fadeIn {
    0% {
        opacity: 0
    }

    to {
        display: block;
        opacity: 1
    }
}

.test {
    font-size: 60px
}
<!DOCTYPE html>
<html>

<head>
    <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" integrity="sha384-MCw98/SFnGE8fJT3GXwEOngsV7Zt27NXFoaoApmYm81iuXoPkFOJwJ8ERdknLPMO" crossorigin="anonymous">
    <link rel="stylesheet" type="text/css" href="./style.css">
</head>

<body>
    <h1 id="title">Interest Rate Calculator</h1>
    <div class="container">
        <div class="wrap">
            <label>% INTEREST</label>
            <select class="form-control" style="width: 10%" name="percentage" id="percentage"></select>

            <label> $ AMOUNT</label>
            <input type="number" min="1" max="999999999" name="amount" id="amount" class="form-control" style="width: 10%">
            <label for=""># OF PAYMENTS</label>
            <input type="number" min="1" max="999" id="time" class="form-control" style="width: 10%">
            <button onclick="handler()" target="_top" class="btn btn-light">CALCULATE!</button>
        </div>
    </div>

    <div id="result">
        <p id="show"></p>
    </div>
    <script type="text/javascript" src="./script.js"></script>
</body>

</html>

Improve this answer
\$\endgroup\$
0
3
\$\begingroup\$

in the function calculateInterest() this is a bit strange :

total = total += percent;

it is either:

total += percent;

or:

total = total + percent;
Improve this answer
\$\endgroup\$
4
  • \$\begingroup\$ A review should be a full review and adress all of the issues in the code. \$\endgroup\$
    – FreezePhoenix
    Oct 1, 2018 at 17:07
  • 1
    \$\begingroup\$ I'd say a rewrite without commentary engenders the same critique. But yeah, this answer is best as a comment but I expect the author does not have comment privileges yet. \$\endgroup\$
    – radarbob
    Oct 1, 2018 at 20:54
  • \$\begingroup\$ @freezePhoenix while there isn't a definitive answer on the topic, it seems that the consensus from answers to this meta post is that short answers are acceptable if they would "constitute a reasonable checkin log message"... \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ ♦
    Oct 2, 2018 at 21:36
  • \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ Noted. Will adjust ~~schedule agenda~~ Welcoming Wagon. \$\endgroup\$
    – FreezePhoenix
    Oct 2, 2018 at 22:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.