oninput and onchange not working because of bad coding

Question

My code is here. Below is the HTML:

<!DOCTYPE html>
<html>
<title>Electricity and Magnetism Demo</title>

<body>
<p>
  <label>Voltage:</label>
  <input id="inputVoltage" type="number" oninput="EqualsVoltage()" onchange="EqualsVoltage()"> </p>

<p>
  <label>Current:</label>
  <input id="inputCurrent" type="number" oninput="EqualsCurrent()" onchange="EqualsCurrent()"> </p>

<p>
  <label>Resistance:</label>
  <input id="inputResistance" type="number" oninput="EqualsResistance()" onchange="EqualsResistance()"> </p>
  <script language="Javascript" type="text/javascript" src="EandM.js"></script>
</body>

</html>

Here is the Javascript:

//Electricity and Magnetism Stuff

function EqualsVoltage() {

    var Voltage = document.getElementById("inputVoltage").value;
    var Current = document.getElementById("inputCurrent").value;
    var Resistance = document.getElementById("inputResistance").value;

    document.getElementById("inputVoltage").value = (Current * Resistance);

}

function EqualsCurrent() {

    var Voltage = document.getElementById("inputVoltage").value;
    var Current = document.getElementById("inputCurrent").value;
    var Resistance = document.getElementById("inputResistance").value;

    document.getElementById("inputCurrent").value = (Voltage / Resistance);

}

function EqualsResistance() {

    var Voltage = document.getElementById("inputVoltage").value;
    var Current = document.getElementById("inputCurrent").value;
    var Resistance = document.getElementById("inputResistance").value;

    document.getElementById("inputResistance").value = (Voltage / Current);

}

I want my calculator to react to both oninput and onchange events when I change a value in the text field.

I've been able to make a converter that converts kilometers to miles when oninput and onchange were functioning; however, I can't figure this out.

When I enter data in the field, it doesn't change the other values. Please help!

no need to pass arguments to functions that don't do anything with the arguments — Jaromanda X
– Jaromanda X, Commented Apr 30, 2019 at 3:32
Just a semantic observation -- the L in some of your getElementById are capitalized. Obviously they should not be. — Christopher Bennett
– Christopher Bennett, Commented Apr 30, 2019 at 3:33
you're also calling "changeVoltage" when you input voltage, therefore, any new value in voltage will be overwritten by calling changeVoltage ... changeVoltage should calculate the OTHER TWO variables (not sure how you can do that, since a change in voltage will result in either a change in current or a change in resistance or both) — Jaromanda X
– Jaromanda X, Commented Apr 30, 2019 at 3:36
This site has some nice simple examples of what you are trying to accomplish. Have a look --- javascriptbank.com/ohm-law-calculator.html — Christopher Bennett
– Christopher Bennett, Commented Apr 30, 2019 at 3:41

Kamil Kiełczewski · Accepted Answer · 2019-04-30 04:42:18Z

4

The problem: when user edit e.g. voltage in input then calculations at the same time change that input value (the input values and calculated values are usually different). Solution: show output calculations in separate place - not as input values. When you use oninput you don't need to use onchange.

function calc() {
  let c = inputCurrent.value;
  let r = inputResistance.value;
  let v = inputVoltage.value;

  msg.innerHTML = `voltage:    ${ c*r } <br>`
                + `current:    ${ v/r } <br>`
                + `resistance: ${ v/c } <br>`;
}

<p>
  <label>Voltage:</label>
  <input id="inputVoltage" type="number" oninput="calc()">
</p>

<p>
  <label>Current:</label>
  <input id="inputCurrent" type="number" oninput="calc()"> 
</p>

<p>
  <label>Resistance:</label>
  <input id="inputResistance" type="number" oninput="calc()"> 
</p>

Calculations:
<div id="msg"></div>

edited Apr 30, 2019 at 4:42

answered Apr 30, 2019 at 4:15

Kamil Kiełczewski

93.7k34 gold badges401 silver badges375 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Comet · Accepted Answer · 2019-04-30 05:39:57Z

This works. Should have been checking the other text boxes not the current one. It will keep changing the fields as the user increases or decreases the value.

<!DOCTYPE html>
<html>
    <title>Electricity and Magnetism Demo</title>
    <head>
        <script type="text/javascript">
            function EqualsVoltage() {

                var Voltage = document.getElementById("inputVoltage").value;
                var Current = document.getElementById("inputCurrent").value;
                var Resistance = document.getElementById("inputResistance").value;
                if(Resistance != "0" && Current != "0" ){
                      document.getElementById("inputVoltage").value = (Current * Resistance);
                }


            }

           function EqualsCurrent() {

                var Voltage = document.getElementById("inputVoltage").value;
                var Current = document.getElementById("inputCurrent").value;
                var Resistance = document.getElementById("inputResistance").value;
                if(Voltage != "0" && Resistance != "0" ){
                    document.getElementById("inputCurrent").value = (Voltage / Resistance);
                }
            }

            function EqualsResistance() {

                var Voltage = document.getElementById("inputVoltage").value;
                var Current = document.getElementById("inputCurrent").value;
                var Resistance = document.getElementById("inputResistance").value;

                if(Voltage != "0" && Current != "0" ){
                    document.getElementById("inputResistance").value = (Voltage / Current);
                }

           }
 </script>
 </head>
 <body>
     <p>
          <label>Voltage:</label>
         <input id="inputVoltage" type="number" oninput="EqualsResistance(); EqualsCurrent()" value="0"> </p>

     <p>
        <label>Current:</label>
        <input id="inputCurrent" type="number" oninput="EqualsVoltage(); EqualsResistance()" value="0"> </p>

     <p>
       <label>Resistance:</label>
        <input id="inputResistance" type="number" oninput="EqualsCurrent();EqualsVoltage()" value="0"> </p>

    </body>

</html>

Kamil Kiełczewski · Accepted Answer · 2019-04-30 04:57:01Z

There is already a great answer you can take, but I wanted to provide you an alternative. It's up to you which one fits better to your needs.

This solution provides an alternative for the user to decide when to calculate the values. This can avoid unexpected values as Infinity, 0, etc..

For this, you could give a button to every element in order to let the user click the one he wants the result for. This will update the value to the input box where he presses the button. The button would look like this:

    <p>
     <label>Resistance:</label>
     <input id="inputResistance" type="number"> 
     <button id="calcResistance"><!-- Add this to every input -->
       Calc
     </button>
    </p>

And your JavaScript code will look like this:

function updateValues(e) {
    let changed = e.target.id,
        Voltage = Number(document.getElementById('inputVoltage').value),
        Current = Number(document.getElementById("inputCurrent").value),
        Resistance = Number(document.getElementById('inputResistance').value);

      switch(changed){
      case "calcResistance":
        document.getElementById("inputResistance").value = (Voltage / Current);
      break;

      case "calcVoltage":
        document.getElementById("inputVoltage").value = (Current * Resistance);
      break;

      case "calcCurrent":
        document.getElementById("inputCurrent").value = Voltage / Resistance;
      break;
    }

}

document.querySelectorAll("button").forEach(b=>b.addEventListener("click",updateValues));

I hope this gives you another way to achieve what you want.

Here is a fiddle of what I am talking about:

function updateValues(e) {
	let changed = e.target.id,
   		Voltage = Number(document.getElementById('inputVoltage').value),
  		Current = Number(document.getElementById("inputCurrent").value),
  		Resistance = Number(document.getElementById('inputResistance').value);
	
	  switch(changed){
      case "calcResistance":
        document.getElementById("inputResistance").value = (Voltage / Current);
      break;

      case "calcVoltage":
        document.getElementById("inputVoltage").value = (Current * Resistance);
      break;

      case "calcCurrent":
        document.getElementById("inputCurrent").value = Voltage / Resistance;
      break;
  	}

}

document.querySelectorAll("button").forEach(b=>b.addEventListener("click",updateValues));

<p>
    <label>Voltage:</label>
    <input id="inputVoltage" type="number"> 
    <button id="calcVoltage">
      Calc
    </button></p>

    <p>
    <label>Current:</label>
    <input id="inputCurrent" type="number"> 
    <button id="calcCurrent">
      Calc
    </button></p>

    <p>
    <label>Resistance:</label>
    <input id="inputResistance" type="number"> 
    <button id="calcResistance">
      Calc
    </button></p>

Very nice idea to change "direction" of UX : not change value after input change, but set value after button click (which seem to be more "natural": first we type values on eg. resistance and current and then push button on voltage to calc its value) +1

Rickard Elimää · Accepted Answer · 2019-04-30 08:08:03Z

0

I would recommend using onkeyup instead of onchange and oninput. [edit] Wont work with the buttons, however.

I'm guessing the workflow is "When I enter something two number fields, the third one is calculated".

But what happens then if all three text fields are filled in?

If you filled in voltage, current, and resistance, and then change voltage again, should current or resistance change?

You need to think about workflow before you do any coding, and that's why the code is wrong.

edited Apr 30, 2019 at 8:08

answered Apr 30, 2019 at 4:41

Rickard Elimää

7,6763 gold badges18 silver badges36 bronze badges

5 Comments

Rickard Elimää Over a year ago

Please do comment if you vote something down. The answer I made is basically talking about doing the foundations of programming right, not providing a finished code. I would had done that, unless I had stepped into this flaw in thinking in this assignment.

Comet Over a year ago

Would using the buttons on the field cause a keyup event? Further, when changing the fields with oninput event, the fields change as the user inputs, a case that "all the fields were changed at the same time" would be irrelevant?

Rickard Elimää Over a year ago

Good one about the buttons. I didn't think of that. Regarding fields changing at the same time, that's not what I meant, but I can't explain it better than I actually did. If voltage is changed, then current and resistance cannot change at the same time, because current is depending on resistance, and resistance on current. Either current or resistance must remain static.

Kaiido Over a year ago

What's that idea of using onkeyup here? Why not onmousemove when you're at it? Or even setTimeout(fn, Math.random() * Infinity)? It will not only fail with the buttons, but with right-click + paste, with assistive devices, will fire unnecessarily on shift press etc.

Comet Over a year ago

ah, I understand. yes this was my issue when coming up with a solution. I covered this as well. +1

Collectives™ on Stack Overflow

oninput and onchange not working because of bad coding

4 Answers 4

Comments

1 Comment

1 Comment

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

1 Comment

1 Comment

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related