0

I've been trying to make the following code to output a value based on the number allocated, to a span. I'm not sure why or how I'm going wrong.

function myFunction() {
  var league = 1
  var shirt = 2

  if (league == 1) {
    if (shirt == 1) {
      document.getElementById("shirt").innerHTML = "Score 70+ Goals = 7,500,000";
    } else if (shirt == 2) {
      document.getElementById("shirt").innerHTML = "Score 60+ Goals = 6,000,000";
    } else if (shirt == 3) {
      document.getElementById("shirt").innerHTML = "Score 55+ Goals = 5,500,000";
    } else if (shirt == 4) {
      document.getElementById("shirt").innerHTML = "Score 50+ Goals = 5,000,000";
    } else if (shirt == 5) {
      document.getElementById("shirt").innerHTML = "Score 45+ Goals = 4,500,000";
    } else if (shirt == 6) {
      document.getElementById("shirt").innerHTML = "Score 40+ Goals = 4,000,000";
    } else if (shirt == 7) {
      document.getElementById("shirt").innerHTML = "Score 35+ Goals = 3,000,000";
    }
  } else if (league == 2) {
    if (shirt == 1) {
      document.getElementById("shirt").innerHTML = "Score 70+ Goals = 7,000,000";
    } else if (shirt == 2) {
      document.getElementById("shirt").innerHTML = "Score 60+ Goals = 5,500,000";
    } else if (shirt == 3) {
      document.getElementById("shirt").innerHTML = "Score 55+ Goals = 5,000,000";
    } else if (shirt == 4) {
      document.getElementById("shirt").innerHTML = "Score 50+ Goals = 4,500,000";
    } else if (shirt == 5) {
      document.getElementById("shirt").innerHTML = "Score 45+ Goals = 4,000,000";
    } else if (shirt == 6) {
      document.getElementById("shirt").innerHTML = "Score 40+ Goals = 3,500,000";
    } else if (shirt == 7) {
      document.getElementById("shirt").innerHTML = "Score 35+ Goals = 2,500,000";
    }
  } else if (league == 3) {
    if (shirt == 1) {
      document.getElementById("shirt").innerHTML = "Score 70+ Goals = 6,500,000";
    } else if (shirt == 2) {
      document.getElementById("shirt").innerHTML = "Score 60+ Goals = 5,000,000";
    } else if (shirt == 3) {
      document.getElementById("shirt").innerHTML = "Score 55+ Goals = 4,500,000";
    } else if (shirt == 4) {
      document.getElementById("shirt").innerHTML = "Score 50+ Goals = 4,000,000";
    } else if (shirt == 5) {
      document.getElementById("shirt").innerHTML = "Score 45+ Goals = 3,500,000";
    } else if (shirt == 6) {
      document.getElementById("shirt").innerHTML = "Score 40+ Goals = 3,000,000";
    } else if (shirt == 7) {
      document.getElementById("shirt").innerHTML = "Score 35+ Goals = 2,000,000";
    }
  }

}
<span id="shirt">Placeholder text</span>
Paul Roub
  • 35,100
  • 27
  • 72
  • 83
HDKing
  • 3
  • 1
  • 2
    Are you actually calling `myFunction` anywhere? – Jared Smith Jul 31 '17 at 21:35
  • The existing of your elements on the page at the time of the script running depends on if you either call the function `myFunction` in a `window.onload` event handler or at the end of the script. If you try to call it in the head without wrapping the function the elements may not exist. Do see any errors in console? – scrappedcola Jul 31 '17 at 21:38
  • You might want to familiarize yourself with the [DRY principle](https://en.wikipedia.org/wiki/Don't_repeat_yourself). Don't worry, my code used to look like this too when I first started programming. – Patrick Roberts Jul 31 '17 at 21:41

4 Answers4

1

As Martin explained in his answer, you're missing a function call. You can call your function either by calling it in your JavaScript code once you've defined your function (i.e., include myFunction(); at the end of your code, as Martin did), or you can add a button to your HTML code that will call your function when clicked, like so:

<button onclick="myFunction()">Click me!</button>

You may also be missing a link to your script. Assuming that your script is in a different file called script.js, you would need to add this tag to the body of your HTML document:

<script src="script.js"></script>

Here are a few quick tips that you can also use to clean up your code:

Don't repeatedly call document.getElementById(). Doing this will instruct JavaScript to repeatedly fetch the #shirt element. You might be better off setting your element to a variable at the start of your function:

var shirtElem = document.getElementById("shirt");

You would then use shirtElem.innerHTML instead of document.getElementById("shirt").innerHTML.

Use switch cases to make your code more readable. This is completely optional, but you can look into the documentation for switch statements here. Your if statements would look something like this instead:

switch (shirt) {
    case 1:
        // do stuff
        break;
    case 2:
        // do stuff
        break;
    // etc.
}

Make your innerHTML strings less repetitive. An easy way to do this is to use a simple function that will produce those strings using a template string. You also might be able to use some variables to save the numbers you're using, and then decrement them according to the pattern you seem to be using. Click here for a rewrite (on Codepen) just for league 1, also using the other suggestions I've specified.

jmindel
  • 335
  • 5
  • 14
  • That's perfect. Just what I was after. I also made the mistake of forgetting to put the brackets after myFunction in . Switch statements will make it a lot easier in the long run too. – HDKing Aug 01 '17 at 09:03
0

There is probably nothing that executes your function. You have various options based on your use case. You could for example run function after definition, or call function from onclick method.

 function myFunction()
 {
  var league = 1      
  var shirt = 2
  
  if (league == 1) {
   if (shirt == 1) {
    document.getElementById("shirt").innerHTML = "Score 70+ Goals = 7,500,000";
   }
   else if (shirt == 2) {
    document.getElementById("shirt").innerHTML = "Score 60+ Goals = 6,000,000";
   }
   else if (shirt == 3) {
    document.getElementById("shirt").innerHTML = "Score 55+ Goals = 5,500,000";
   }
   else if (shirt == 4) {
    document.getElementById("shirt").innerHTML = "Score 50+ Goals = 5,000,000";
   }
   else if (shirt == 5) {
    document.getElementById("shirt").innerHTML = "Score 45+ Goals = 4,500,000";
   }
   else if (shirt == 6) {
    document.getElementById("shirt").innerHTML = "Score 40+ Goals = 4,000,000";
   }
   else if (shirt == 7) {
    document.getElementById("shirt").innerHTML = "Score 35+ Goals = 3,000,000";
   }
 }

  else if (league == 2) {
   if (shirt == 1) {
    document.getElementById("shirt").innerHTML = "Score 70+ Goals = 7,000,000";
   }
   else if (shirt == 2) {
    document.getElementById("shirt").innerHTML = "Score 60+ Goals = 5,500,000";
   }
   else if (shirt == 3) {
    document.getElementById("shirt").innerHTML = "Score 55+ Goals = 5,000,000";
   }
   else if (shirt == 4) {
    document.getElementById("shirt").innerHTML = "Score 50+ Goals = 4,500,000";
   }
   else if (shirt == 5) {
    document.getElementById("shirt").innerHTML = "Score 45+ Goals = 4,000,000";
   }
   else if (shirt == 6) {
    document.getElementById("shirt").innerHTML = "Score 40+ Goals = 3,500,000";
   }
   else if (shirt == 7) {
    document.getElementById("shirt").innerHTML = "Score 35+ Goals = 2,500,000";
   }
 }

  else if (league == 3) {
   if (shirt == 1) {
    document.getElementById("shirt").innerHTML = "Score 70+ Goals = 6,500,000";
   }
   else if (shirt == 2) {
    document.getElementById("shirt").innerHTML = "Score 60+ Goals = 5,000,000";
   }
   else if (shirt == 3) {
    document.getElementById("shirt").innerHTML = "Score 55+ Goals = 4,500,000";
   }
   else if (shirt == 4) {
    document.getElementById("shirt").innerHTML = "Score 50+ Goals = 4,000,000";
   }
   else if (shirt == 5) {
    document.getElementById("shirt").innerHTML = "Score 45+ Goals = 3,500,000";
   }
   else if (shirt == 6) {
    document.getElementById("shirt").innerHTML = "Score 40+ Goals = 3,000,000";
   }
   else if (shirt == 7) {
    document.getElementById("shirt").innerHTML = "Score 35+ Goals = 2,000,000";
   }
 }

 }
   // If you would like to execute function instantly after its definition
   //myFunction(); 
<span id="shirt">Placeholder text</span>
<button onclick="myFunction()">Press me!</button>
Andrew Dunai
  • 2,784
  • 16
  • 26
Martin Hlavňa
  • 592
  • 5
  • 19
0

How are you calling myFunction? Perhaps you can put an alert(!!document.getElementById("shirt")) in there? That way you will know for sure if the function is running or not, and if it alerts true you will know that your element is ready.

Two things that might be going wrong:

  1. Your function is not running at all
  2. Your function is running before the shirt element is on the page, because the script is above the definition of that span and it runs synchronously

The solutions to them are:

  1. Call your function
  2. Move the script tag to the bottom of the page, or see this answer
NotJavanese
  • 2,862
  • 1
  • 13
  • 15
0

you are defining the function but the function is not triggered by any event or element try

    <span id="shirt">Placeholder text</span>
<input id="clickMe" type="button" value="clickme" onclick="myFunction();" />

you can try with a switch instead of many else