sclark
sclark

Reputation: 43

Cannot get alert to display correctly when using an if statement

I was able to get a different version of this code working with help I received here last week. I am learning so I decided to refactor it using a for loop to go through the array this time. That in itself seems to work fine until I add an 'else' to alert the user that they entered incorrect data.

I see that being inside the for loop it loops with the number of times it iterates, but no matter where I place the alert it just seems to override everything. It will pop up first no matter what is entered, then you have to click it 5 times before it displays the data in the array. I'm open to other methods, not set on using an alert only.

I tried writing a function like this so I could just move the call around to test:

    function inValidAlert(){
     let cylFindInvalid = 
     document.getElementById("cylEnter").value.toUpperCase();
     if(cylFindInvalid != cylArray.name){
        let displayIt = document.getElementById("displayCyl");
        displayIt.innerHTML = displayIt.innerHTML + "Enter a valid 
        cylinder type";
      }
    }

I placed invalidAlert() inside the listCylinders function, outside the function, etc...That gave me the same results as just writing an alert, but gave me some practice anyway...

This might seem like a no brainer to some of you, but I am learning Javascript and need help to figure this one out:)

The code shown works without the alert statement. So a user enters a cylinder type with the 'name' property of one in the array. LD, RD, GD, etc... If it is valid then the data in the object (in the array) displays on the screen. But if an invalid entry is seen, I want a pop up alert to show that says "Invalid Data" or similar. The pop up will work but at the wrong time or taking multiple clicks to clear it if inside the for loop. If I use a 'break' then the alert overrides the entire if statement and will fire no matter what is entered.

So how would I go about getting the alert to fire properly? Maybe the for loop is the wrong overall approach to begin with? Let me know if I need to post my HTML also. I am new and trying to learn the ropes here, please go easy on me.

function listCylinders() {
  let display = document.getElementById("displayCyl");
  for (var i = 0; i < cylArray.length; i++) {
    let cylCompare = document.getElementById("cylEnter").value.toUpperCase();
    if (cylCompare == cylArray[i].name) {
      display.innerHTML = display.innerHTML + cylArray[i].name + ":" + "<br>" + cylArray[i].brand +
        "<br>" + cylArray[i].cylinder + "<br>" + cylArray[i].pins + "<br>" + cylArray[i].type;
    } else {
      alert("Enter a valid cylinder type");
    }
  }
}
//function used to disable the button after it is used once.
const setbutton = document.getElementById('button1');
setbutton.addEventListener('click', disableButton);

function disableButton() { 
  setbutton.disabled = true;
}
//function that will clear the form as well as the display when clicked.

function clearCyl() {
  var cylDisplay = document.getElementById("displayCyl");
  document.getElementById("cylForm").reset();
  cylDisplay.innerHTML = "";
  setbutton.disabled = false;
}
//cylinder type properties shown as individual objects inside of an array.
var cylArray = [{
    name: 'LD',
    brand: 'Schlage, Falcon',
    cylinder: ' Without cylinder',
    pins: ' 6 Pin',
    type: ' KIL'
  },
  {
    name: 'RD',
    brand: 'Schlage-Everest 29 S123 (Standard)',
    cylinder: ' With cylinder',
    pins: ' 6 Pin',
    type: ' FSIC'
  },
  {
    name: 'PD',
    brand: 'Schlage-Everest 29 S123 (Standard)',
    cylinder: ' With cylinder',
    pins: ' 6 Pin',
    type: ' KIL'
  },
  {
    name: 'JD',
    brand: 'Schlage',
    cylinder: ' Without cylinder',
    pins: ' 6 Pin',
    type: ' FSIC'
  },
  {
    name: 'GD',
    brand: 'Schlage-Everest 29 R Family keyways',
    cylinder: ' With cylinder',
    pins: ' 7 Pin',
    type: ' SFIC'
  }
];

Upvotes: 0

Views: 50

Answers (1)

Spencer Wieczorek
Spencer Wieczorek

Reputation: 21575

In your loop logically it will always alert at least 5 times. This is because cylCompare never changes, so it will never be equal to the other 5 .name properties in the array you loop. So cylCompare == cylArray[i].name can only ever be true for one of your six iterations. Hence the alert showing 5 times in a row.

Instead what you will want to do is if the user input never matches a single item in your array, then you will want to say it's invalid. You can do this by assuming it's invalid first, then once you find any match change it to being valid:

function listCylinders() {
  let display = document.getElementById("displayCyl");
  let valid = false; // We assume this isn't valid
  // You can move this outside of the loop, and trim in case the user entered spaces
  let cylCompare = document.getElementById("cylEnter").value.toUpperCase().trim();
  for (var i = 0; i < cylArray.length; i++) {
    if (cylCompare == cylArray[i].name) {
      valid = true; // We find a match in one of our array items, so it's valid
      display.innerHTML = display.innerHTML + cylArray[i].name + ":" + "<br>" + cylArray[i].brand +
        "<br>" + cylArray[i].cylinder + "<br>" + cylArray[i].pins + "<br>" + cylArray[i].type;
    }
  }
  if(!valid) alert("Enter a valid cylinder type"); // If we never found a match then we alert
}

Upvotes: 1

Related Questions