Trying to understand JS functions - what am I doing wrong?

Question

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 which converts human years to dog years. Here is my code so far:

HTML:

<div id="calculator">
  <form>
    <label>What is your current age in human years? <br>
    <input type="text" id="humanYears"></label> <br>
    <button type="text" id="calculate">Calculate</button>
  </form>
</div>

JS:

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

document.getElementById("calculate").onclick = function(){calculate(); };

The page flickers and I keep seeing the form, no result.

I know this code is incorrect but I don't understand why. I also know I can just copy other people's code from Github and have a functioning calculator but that kind of defeats the purpose of learning. I would rather know why my code doesn't work and what I can do to fix it. (I double, triple checked that the HTML and JS files were properly linked, which they are.)

Any JS wizards out there care to chime in?

Edit: When I enter an age into the form, it merely reloads, rather than displaying the age in dog years (which is the desired outcome).

Answer 1

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

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

document.getElementById("calculate").onclick = function(){calculate(); };

<div id="calculator">
  <form>
    <label>What is your current age in human years? <br>
    <input type="text" id="humanYears"></label> <br>
    <button type="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');

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

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

form.addEventListener('submit', calculate, false);

<form id="calculator">
  <p>
    <label>
      What is your current age in human years?<br>
      <input type="text" name="humanYears">
    </label>
  </p>
  <p>
    <button type="submit">Calculate</button>
  </p>
  <p>
    Answer: <span id="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

Answer 2

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:

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

document.getElementById("calculate").onclick = calculate;

<div id="calculator">
  <form>
      <label>What is your current age in human years? <br>
          <input type="text" id="humanYears">
      </label>
      <br>
      <button type="button" id="calculate">Calculate</button>
      <div id="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.

Answer 3

Here your working code with as little changes as possible:

<div id="calculator">
  <form>
    <label>What is your current age in human years? <br>
    <input type="text" id="humanYears"></label> <br>
    <button type="text" id="calculate">Calculate</button>
  </form>
</div>

<script>
  function calculate() {
    var humanYears = document.getElementById("humanYears").value;
    var dogYears = (humanYears * 7);
    document.write(dogYears);
  }

  document.getElementById("calculate").onclick = function(){calculate(); return false; };
</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.