views:

152

answers:

8

I'm using a for loop to cycle through some elements with a starting value (seats on a plane).

Here it is:
seatNum - Number of seats to be cycled through
startSeat - seat to begin cycling

I'm calling the function from a form "onsubmit".

The problem comes in the for loop when i try and go get elements with an id naming convention of "s1" "s2" "s3" etc... "s45" "s46" etc... based on the loop counter added to the starting seat. Counting from 0(starting seat) up to seatNum(how many seats).

any idea why by id isn't resolving properly? All others work fine except the last one inside the for loop.

Yes I'm new to programming so i probably don't have the best practices, please be forgiving stylistically.

function check() {
    var startSeat;
    var fName = document.getElementById('fName').value
    var lName = document.getElementById('lName').value
    var address = document.getElementById('address').value
    var city = document.getElementById('city').value
    var state = document.getElementById('state').value
    var zip = document.getElementById('zip').value
    var phone = document.getElementById('phone').value
    var seatNum = document.getElementById('seatNumber').value
    var y=document.getElementById('seatList1').value;
    var z=document.getElementById('seatList2').value;

    if (z >= y) {
        startSeat = y;
    }
    else {
        startSeat = z;
    }

    if ( (fName == "") || (lName == "") || (address == "") || (phone == "") || (zip == "") || (state == "") || (city == "") ) {
        alert("You must fully complete the form");
        return false;
    }

    for (var i = 0; i < seatNum; i++) {
        if (document.getElementById("s"+(startSeat+i)).className=="taken"){
            alert("Selected seat(s) already booked.");
            return false;
        }
    else {
            continue;
        }
    }
}
+2  A: 

Try this:

for (var i = startSeat; i < seatNum; i++) {
    if (document.getElementById("s"+i).className == "taken") {
     alert("Selected seat(s) already booked.");
     return false;
    }
}

Rather than adding i to the startSeat value to get the seat id, use startSeat's value right in the loop initialization. I believe what was happening is that you were getting an off-by-one error since your startSeat value was already set and then you were adding i to it which bumped you ahead by one.

Andrew Hare
the problem is that startSeat can be numbers ranging from 1-104, and seatNum will most always be a single digit, smaller number. so the condition (i<seatNum) above will always fail and skip the loop all together. good thought though.
jon
+5  A: 

I'm not sure, but maybe startSeat+i concats the strings and not doing the mathematical add you expect. Try alerting to screen:

alert(document.getElementById("s"+(startSeat+i)));

Is it the field name?

Faruz
A: 

You could be iterating one too many times. Try i < seatNum - 1 in your for loop

Kenaniah
+7  A: 

Convert your y and z variables to number:

var y = +document.getElementById('seatList1').value;
var z = +document.getElementById('seatList2').value;

var startSeat = (z >= y) ? y : z; // or simply startSeat = Math.min(z,y);

That will fix the problem that @Faruz pointed out.

CMS
Thanks. I can tell this site is going to be a problem for me. :)
jon
+1  A: 

When you say cycle through the seats, I assume you want to continue counting from 1 after you reach the maximum seat? Assuming that numberOfSeats is defined somewhere (I couldn't see it, but you must have it somewhere) you can do this:

"s"+((startSeat + i - 1) % numberOfSeats + 1)

so the line in full:

if (document.getElementById("s"+((startSeat + i - 1) % numberOfSeats + 1)).className=="taken"){
Mark Byers
+1  A: 

What you're really getting is string concatenation, so what is really happening is this:

i = 10
seatNum = 1

(seatNum+i) = "110"

Try this using the parseInt() function for casting the variables to integer types:

if (document.getElementById("s"+(parseInt(startSeat)+parseInt(i))).className=="taken")
jaywon
+1  A: 

Looks to me like startSeat is a string type. Even though JavaScript is type-less, the value of a DOM object is going to default as a string. So, you're getting a concatenation instead of addition.

Use what CMS wrote. That should fix your problem.

Jeff Rupert
A: 

Debug first. Instead of:

alert("Selected seat(s) already booked.");

Use:

alert("s"+(startSeat+i));

See if this evals to a valid <div> element. Like other's said, the error is probably related to type-casting.

Salman A