null/undefined checking for checking seats

This looks pretty messy (need to reduce nesting I feel). I need a check an input for a seat, but I can't guarantee it has a value (one may not be chosen for example). If the seats aren't in a certain color it goes to seat basic (1), otherwise seat premium (2).

var seat = $("input[id*=" + seatPrefix + "]");
    if (seat != 'undefined') {
        seat = seat[0].value;
        if (seat != "") {
            if (seat != "red" && seat != "blue" && seat != "silver" && seat != "gold") {
                chooseSeat(seat, "2");
            }
            else {
                chooseSeat(seat, "1");
            }
        }
    }

var seat = $("input[id*=" + seatPrefix + "]");
    if (seat != 'undefined') {

First off, an extra level of indentation has crept in here. Let me assume this is just a copy/paste error.

Any chance you could be using classes here instead of prefixed IDs? Best case: a single ID you know in advance. It seems that you assume there's only one match anyways.

jQuery, when it doesn't find anything, returns an empty collection, not undefined (or 'undefined'; did you get confused by typeof x !== 'undefined'?). To test if a jQuery collection is empty, you can use seat.length === 0 or even ! seat.length.

I don't normally use hungarian notation, but I do use it for jQuery objects, especially when I'm dealing with native elements as well: $seat. On a side note, shouldn't the variable name be $seatInput or something, rather than just $seat?

I recommend against the coercing equality operator (==). Use === instead. == can be a bit... unpredictable at times. There are some special cases or case where I find == acceptable (== null for null or undefined) but this is not one of them (if only because undefined != 'undefined')

seat = seat[0].value;

You can use seat.val() here. This has a positive side-effect that val returns undefined if the jQuery object holds no elements. Your code will attempt to dereference undefined in that case.

Also, you are reusing the same variable to mean a jQuery object at one point, then to mean a string in the next line. You should use two separate variables (or inline the first one) here.

if (seat != "") {

If you use val, you can move it outside its condition block, and merge the condition with this one:

if(seat !== undefined && seat !== "")

Since both undefined and the empty string are falsy and all other strings are truthy, this will work as well:

if(seat)

Of course, @retailcoder's suggestion to invert the condition and return early still applies.

if (seat != "red" && seat != "blue" && seat != "silver" && seat != "gold") {

You can use indexOf to shorten that code and make it more readable:

if (["red", "blue", "silver", "gold"].indexOf(seat) == -1)

if you like shortcuts,

if (!~["red", "blue", "silver", "gold"].indexOf(seat))

At this point, the array definition can (and should) be moved outside the condition. It is marginally nicer to the memory, but more importantly it's easier to find the array in case you want to ever change it if you put it at the beginning of the file.

if(...){
    chooseSeat(seat, "2");
} else {
    chooseSeat(seat, "1");
}

Shouldn't this logic be part of the chooseSeat function?

Also, 1 and 2 are non-obvious. Since you're passing a string anyways (why?), perhaps chooseSeat should accept "basic" and "premium" as its arguments?

If I couldn't modify the chooseSeat function or your HTML, I would probably refactor your code like this:

var basicColors = ["red", "blue", "silver", "gold"];
var seat = $("input[id*=" + seatPrefix + "]").val();
if (!seat) return;

if (~basicColors.indexOf(seat)) {
    chooseSeat(seat, "1");
} else {
    chooseSeat(seat, "2");
}

or, if return cannot be used (this is not the whole body of the function it is in),

var basicColors = ["red", "blue", "silver", "gold"];
var seat = $("input[id*=" + seatPrefix + "]").val();

if (seat) {
    if (~basicColors.indexOf(seat)) {
        chooseSeat(seat, "1");
    } else {
        chooseSeat(seat, "2");
    }
}

StackExchange Code Review Q#36582, answer score: 12