Why does this not function correctly?

No matter what value you enter it ALWAYS returns "Invalid Ticket Type"

function start() {
  var TickType;
  TickType = document.getElementById("TicketType").value;
  TickType = String(TickType);
  var TicketQty = document.getElementById("TicketQty").value;

  if (TickType != "A" || TickType != "B" || TickType != "C") {
    document.getElementById("msg").innerHTML = "Invalid Ticket Type";
  }

  if (isNaN(TicketQty)) {
    document.getElementById("msg").innerHTML = "Non numeric qty has been entered";
  }

  if (TicketQty < 1 || TicketQty > 100) {
    document.getElementById("msg").innerHTML = "Qty is outside valid range";
  }
}

<h1>Ticket Sales</h1>
<p>Enter the ticket type (A, B or C)</p>
<input type="text" id="TicketType">

<p>Enter the quantity required (1-100)</p>
<input type="text" id="TicketQty">

<p>
  </br>
</p>

<button onclick="start()">Click me</button>
<p id="msg"></p>

Why does this not function correctly?

No matter what value you enter it ALWAYS returns "Invalid Ticket Type"

function start() {
  var TickType;
  TickType = document.getElementById("TicketType").value;
  TickType = String(TickType);
  var TicketQty = document.getElementById("TicketQty").value;

  if (TickType != "A" || TickType != "B" || TickType != "C") {
    document.getElementById("msg").innerHTML = "Invalid Ticket Type";
  }

  if (isNaN(TicketQty)) {
    document.getElementById("msg").innerHTML = "Non numeric qty has been entered";
  }

  if (TicketQty < 1 || TicketQty > 100) {
    document.getElementById("msg").innerHTML = "Qty is outside valid range";
  }
}

<h1>Ticket Sales</h1>
<p>Enter the ticket type (A, B or C)</p>
<input type="text" id="TicketType">

<p>Enter the quantity required (1-100)</p>
<input type="text" id="TicketQty">

<p>
  </br>
</p>

<button onclick="start()">Click me</button>
<p id="msg"></p>

• javascript
• html
• if-statement
• or-condition

• 6 Use && instead of ||. – Tushar Commented Apr 20, 2017 at 5:47
• 1 use TickType.toString() instead of String(TickType) – Jrey Commented Apr 20, 2017 at 5:49
• Please see the snippet I made for you. I clicked the <> and created a minimal reproducible example – mplungjan Commented Apr 20, 2017 at 5:56
• @JohnReyM.Baylen or nothing since it is already a string, especially if OP adds value="" to each field as remended. - A .trim() is a better idea – mplungjan Commented Apr 20, 2017 at 5:57

5 Answers 5

Use && instead of ||

if (TickType != "A" || TickType != "B" || TickType !=  "C"){
    document.getElementById("msg").innerHTML = "Invalid Ticket Type";
}

Explanation

When TickType = "A", TickType != "B" and TickType != "C" conditions are true.

Here two of the condition is always true, so it goes into the if statement

NOTE: Add a trim and a parseInt to the vars and value="" to the fields and emptied the message before testing

function start() {
  var TickType = document.getElementById("TicketType").value.trim();
  var TicketQty = parseInt(document.getElementById("TicketQty").value, 10);
  document.getElementById("msg").innerHTML = "";

  if (TickType != "A" && TickType != "B" && TickType != "C") {
    document.getElementById("msg").innerHTML = "Invalid Ticket Type";
  }

  if (isNaN(TicketQty)) {
    document.getElementById("msg").innerHTML = "Non numeric qty has been entered";
  }

  if (TicketQty < 1 || TicketQty > 100) {
    document.getElementById("msg").innerHTML = "Qty is outside valid range";
  }
}

<h1>Ticket Sales</h1>
<p>Enter the ticket type (A, B or C)</p>
<input type="text" id="TicketType" value="" />

<p>Enter the quantity required (1-100)</p>
<input type="text" id="TicketQty" value="" />

<p><br/></p>

<button onclick="start()">Click me</button>
<p id="msg"></p>

Lets try to figure out why its not working correctly

You expect TickType to be either A or B or C. Now when you evaluate following condition, lets say TickType="A"

if (TickType != "A" || TickType != "B" || TickType !=  "C"){

• TickType != "A" would yield false as it is equal.
• But then TickType != "B" would yield true as TickType is A and not B

and if finds matched condition and sets innerHTML as "Invalid Ticket Type"

Now the correct condition would be if its not in either value i.e.

if (TickType != "A" && TickType != "B" && TickType !=  "C"){

Now this will work but what if you have say 10 such characters in future, adding 10 such && condition would make code less readable.

Alternatives

1. Create an array with all such possibilities and check for index.

var possibilities = ["A", "B", "C", "D", "E"];

function isAvailable(c){
  return possibilities.indexOf(c) > -1
}

console.log(isAvailable("G"))
console.log(isAvailable("A"))

2. Create a regex and test for validity:

var possibilities = /(A|B|C|D|E)/

function isAvailable(c){
  return possibilities.test(c)
}

console.log(isAvailable("G"))
console.log(isAvailable("A"))

The problem is here ...

if (TickType != "A" || TickType != "B" || TickType !=  "C"){
    document.getElementById("msg").innerHTML = "Invalid Ticket Type";
}

This condition always evaluates to true, even if you enter any of the A,B,C. For instance, you enter an A, the first inequality fails, which is ok, but all the other will succeed and you'll get the "Invalid Ticket".

The logic says that:

!(A || B || C) <-> !A && !B && !C

so change that if-condition to:

if (TickType != "A" && TickType != "B" && TickType !=  "C"){
   document.getElementById("msg").innerHTML = "Invalid Ticket Type";
}

This will do the trick ... If any of the TickType's is A or B or C, the ands will fail and you won't get an "Invalid Ticket"

Try following code.

if (TickType != "A" && TickType != "B" && TickType !=  "C")

You need to use AND instead of OR in condition. Where, you're checking if TicketType isn't one out of A,B & C. So condition will be

if (TickType != "A" && TickType != "B" && TickType !=  "C")
{
    document.getElementById("msg").innerHTML = "Invalid Ticket Type";
}