Skip to content Skip to sidebar Skip to footer

Trying To Understand Js Functions - What Am I Doing Wrong?

I'm currently working my way through a beginner's JavaScript course on Treehouse and keep getting stuck on functions. In effort to understand better, I tried creating a calculator

Solution 1:

Your code works, although as you've indicated it's not great.

functioncalculate() {
 var humanYears = document.getElementById("humanYears").value;
 var dogYears = (humanYears * 7);
 document.write(dogYears);
}

document.getElementById("calculate").onclick = function(){calculate(); };
<divid="calculator"><form><label>What is your current age in human years? <br><inputtype="text"id="humanYears"></label><br><buttontype="text"id="calculate">Calculate</button></form></div>

Some notes for improvement:

  • Avoid document.write
  • Forms should have submit buttons (either <input type="submit" value="Calculate"> or <button type="submit">Calculate</button>
  • The parentheses around your arithmetic are superfluous: var dogYear = humanYears * 7; is sufficient
  • Not everything needs an id attribute, although that makes DOM queries easy and quick
  • You should handle the form's submit event as opposed to the button's click event as you'll want to handle if, say, I submit the form by pressing Enter on my keyboard
  • You don't need the extra function around calculate, document.getElementById('calculate').onclick = calculate; would suffice

With those notes in mind, here's how I'd improve your calculator:

var form = document.getElementById('calculator');

functioncalculate() {
  var years = form['humanYears'].value,
      dogYears = years * 7;

  document.getElementById('answer').innerText = dogYears;  
}

form.addEventListener('submit', calculate, false);
<formid="calculator"><p><label>
      What is your current age in human years?<br><inputtype="text"name="humanYears"></label></p><p><buttontype="submit">Calculate</button></p><p>
    Answer: <spanid="answer"></span></p></form>

Things I've changed:

  • I'm using <p> tags to control whitespace instead of <br> which will further let me customize presentation with CSS if I choose to. You cannot style <br> elements.
  • I'm modifying a portion of the DOM, not the entire DOM
  • I've bound my event handler with addEventListener which is way less obtrusive
  • I'm accessing form elements through the natural structure the DOM provides instead of running a full DOM query for each element
  • I've reduced some code

Solution 2:

Here your working code with as little changes as possible:

<divid="calculator"><form><label>What is your current age in human years? <br><inputtype="text"id="humanYears"></label><br><buttontype="text"id="calculate">Calculate</button></form></div><script>functioncalculate() {
    var humanYears = document.getElementById("humanYears").value;
    var dogYears = (humanYears * 7);
    document.write(dogYears);
  }

  document.getElementById("calculate").onclick = function(){calculate(); returnfalse; };
</script>
  • Assuming you put everything in one file the script tags are missing. If not then you still need a script tag to load the JS file.
  • Your function needed a "return false;". If you omit that, the page will reload after writing your output and won't see the output. That happens because the default behaviour of a button in a form is to reload the page. By returning "false" you suppress that.

Solution 3:

The main problem is that document.write doesn't do what you imagine it does:

Note: as document.write writes to the document stream, calling document.write on a closed (loaded) document automatically calls document.open, which will clear the document.

See the documentation for document.write: https://developer.mozilla.org/en-US/docs/Web/API/Document/write

A better way to this is to have an empty element on the page, which you then change the contents of:

functioncalculate() {
    var humanYears = document.getElementById("humanYears").value;
    var dogYears = humanYears * 7;
    document.getElementById('output').innerText = dogYears;
}

document.getElementById("calculate").onclick = calculate;
<divid="calculator"><form><label>What is your current age in human years? <br><inputtype="text"id="humanYears"></label><br><buttontype="button"id="calculate">Calculate</button><divid="output"></div></form></div>

I've also made some small improvements to your script:

  • Changed the indentation of your HTML to be more readable
  • Changed your button to have type="button" - otherwise your form will submit and the page will reload when you click the button. In this case, you actually don't even need a form element, but it's not hurting anything. Alternatively, you could add return false to your calculate function - this would tell the browser not to submit the form and thus not reload the page
  • Changed how you're adding the onclick handler - there's no need to wrap the calculate function in another function. In javascript, functions can actually be passed around like a variable. This is why I set the value of onclick to just be calculate - notice however that I left out the (). You want the onclick to be a reference to the function, otherwise the calculate function would be executed immediately, and the onclick would be set to the return value of the function - in this case, that would be undefined.

Post a Comment for "Trying To Understand Js Functions - What Am I Doing Wrong?"