jQuery jump menu

Question

I am trying to make some code shorter and usable. Can someone look over the code I wrote and let me know if it's done correctly? I think it is but it seems a little too simple. The code allows a user to select a location form a drop down, then goes to that page.

Old Code

 if (typeof $.uniform === "undefined") {
 } else {
 $.uniform.update("select.jump");
 $("select#mexico").change(function() {
 window.location = $("select#mexico option:selected").val();
 });
 $("select#paris").change(function() {
 window.location = $("select#paris option:selected").val();
 });
 $("select#london").change(function() {
 window.location = $("select#london option:selected").val();
 });
 $("select#jumpMenu").change(function() {
 window.location = $("select#jumpMenu option:selected").val();
 });
 }

New Code

 if (typeof $.uniform === "undefined") {
 } else {
 $.uniform.update("select.jump");
 $(this).change(function() {
 window.location = $(this).val();
 });
 }

• zerkms: "let me know if its done correctly" --- does it work as expected? If yes - then yes, it's been done correctly. PS: there are != and !== in js

Answer 1

Try this sample

html:

<select>
 <option val="#">---</option>
 <option val="#paris">paris</option>
</select>
<br/>
<select>
 <option val="#">---</option>
 <option val="#mexico">mexico</option>
</select>
<br/>
<select>
 <option val="#">---</option>
 <option val="#london">london</option>
</select>

​js:

//if (typeof $.uniform !== "undefined") {
 //$.uniform.update("select.jump");
 $("select").each(function(){
 $(this).change(function() {
 alert($(this).find("option:selected").val());
 window.location = $(this).find("option:selected").val();
 });
 });
//}​​

uncomment your function parts

Answer 2

I'm not too familiar with jQuery, but instead of the empty block I'd invert the condition:

if (typeof $.uniform !== "undefined") {
 $.uniform.update("select.jump");
 $(this).change(function() {
 window.location = $(this).val();
 });
}

It's definitely more readable. Or you can use a guard clause if it is in a function:

if (typeof $.uniform === "undefined") {
 return;
}
$.uniform.update("select.jump");
$(this).change(function() {
 window.location = $(this).val();
});

(References: Replace Nested Conditional with Guard Clauses in Refactoring: Improving the Design of Existing Code; Flattening Arrow Code)

• latoyale: I did invert the condition. That makes sense. Also thanks for the link about flattening code. I didn't know I had a problem until I read the article. :)