CODEWITHSUNDEEP
javascript
mdebos
mdebos

Reputation: 41

JS to perform calculation based on user input

I have a code where I am trying to calculate a total value based on the value of the input selected by a user. It seems simple but I can't get the total to reflect. Please can someone show me where the fault is?

function calculate() {
  var panel = parseInt(document.getElementsById("panel").value);
  panelv = 65;
  panelt = panel * panelv;
  derating_value = 2;
  total_hours_standby = panelt * derating_value;
}

document.getElementsById("total_hours").innerHTML = total_hours_standby;
<input type="number" id="panel" placeholder="panel quantity"></input><br>
<button type="button" onclick="calculate">Result</button>

<p id="total_hours">result displays here</p>

Upvotes: 2

Views: 2329

Answers (5)

matvs
matvs

Reputation: 1873

There are couple of issues here:

Most you could have found out if you had looked into console logs.
For starter the function is called getElementById not getElementsById, because there is supposed to be only one element with unique id, so plural does not make sense here.

Another one is a logic error: not updating content after clicking on button i.e when calculate gets executed.

There is also one more syntax error, which is how functions should be passed to HTML element's attribute. It needs to be functionName() instead of functionName

This is how simply fixing this code could look like: 

var total_hours_standby = 0;
 function calculate() {
 var panel = parseInt(document.getElementById("panel").value);
 panelv = 65;
 panelt = panel * panelv;
 derating_value = 2;
 total_hours_standby = panelt * derating_value;
 document.getElementById("total_hours").innerHTML = total_hours_standby;
  }

 document.getElementById("total_hours").innerHTML = total_hours_standby;
 <input type="number" id="panel" placeholder="panel quantity"></input><br>
 <button type="button" onclick="calculate()">Result</button>

 <p id="total_hours">result displays here</p>

Here I give you couple of ideas for improving it.

  • Since you use global variable total_hours_standby it may be a good idea to encapsulate it. So called module pattern should do the job.
  • New value of total_hours_standby does not seem to depend on an old one, so I guess you mean to use it somewhere else - in order to do so, you need to expose it with "public" getter.
  • If above is not the case, then you don't need total_hours_standby variable at all and you can just directly return it or display it without storing this value in variable.
  • I put code for rendering in separate function - this is because rule of thumb for functions is that they should have single responsibility. One functions for calculations, another for rendering and then one function for handling user's input and click event, that uses two previous ones. This way if for example you only want to calculate something without rendering result, then you just, simply can :)
  • I also stored DOM nodes in variables, instead of calling getElementById, it is not due to performance, how it is often assumed, I did it only for better readability.
  • Constants instead of hard-coded values.

var Calculator = (function() {
  const panelInput = document.getElementById("panel");
  const output =  document.getElementById("total_hours");

  const PANEL_V = 65;
  const DERATING_VALUE = 2;
  
  const render = value => output.innerHTML = value;
  const calculate = value => value * PANEL_V * DERATING_VALUE;
  
  let total_hours_standby = 0;
  return {
    handleInput: function() {
      total_hours_standby = calculate(panelInput.value);
      render(total_hours_standby);
    },
    getTotalHoursStandby: () => total_hours_standby
  };
})();
<input type="number" id="panel" placeholder="Panel quantity" />
 <button type="button" onclick="Calculator.handleInput()">Calculate</button>

 <p id="total_hours">Result displays here</p>

Upvotes: 0

mplungjan
mplungjan

Reputation: 178422

  1. getElementById is singular
  2. declare your vars
  3. call calculate() with brackets
  4. assign the value inside the function
  5. </input> is not needed

Here is a version with eventListeners since other answers already showed you how to fix YOUR version

function calculate() {
  var panel = +document.getElementById("panel").value;
  if (panel === "" || isNaN(panel)) panel = 0;
  let panelv = 65;
  let panelt = panel * panelv;
  let derating_value = 2;
  document.getElementById("total_hours").textContent = (panelt * derating_value);
}
window.addEventListener("load", function() {
  document.getElementById("calc").addEventListener("click", calculate)
  calculate(); // run at load
})
<input type="number" id="panel" placeholder="panel quantity"><br>
<button type="button" id="calc">Result</button> result displays here: <span id="total_hours"></span>

Upvotes: 1

nu_pagadi
nu_pagadi

Reputation: 53

First, you should call method calculate.

<button type="button" onclick="calculate()">Result</button>

Then, add this line document.getElementsById("total_hours").innerHTML = total_hours_standby; inside calculate function.

Alos, typo error: document.getElementById instead of document.getElementsById

Upvotes: 0

Nina Scholz
Nina Scholz

Reputation: 386868

You need to

  • use for onclick="calculate()" take the function call, not only the function,
  • use getElementById, spelling matter,
  • declare all variables,
  • and finally move the output inside of the function

function calculate() {
    var panel = parseInt(document.getElementById("panel").value),
        panelv = 65,
        panelt = panel * panelv,
        derating_value = 2,
        total_hours_standby = panelt * derating_value;

    document.getElementById("total_hours").innerHTML = total_hours_standby;
}
<input type="number" id="panel" placeholder="panel quantity"></input><br>
<button type="button" onclick="calculate()">Result</button>

<p id="total_hours">result displays here</p>

Upvotes: 3

The KNVB
The KNVB

Reputation: 3844

It is typo,

document.getElementsById

should be

document.getElementById

Upvotes: -1

Related Questions