37
$(document).ready(function(){
    createForm("text,password",".content");
});

function createForm(types,object){
    typ = types.split(',');
    
    //var source = "";
    
    $.each(typ,function(){
        switch(this) {
            case "text":
                console.log('text');
                break;
            default:
                console.log('default');
                break;
        }
    });
    //$(object).html(source);
}

I have this code an in console it return 2xdefaults. Why?

I try to return a input for each type as text or password but my switch does not recognize the "typ"

Reid Moffat
  • 322
  • 1
  • 3
  • 16
kraYz
  • 587
  • 1
  • 4
  • 11

4 Answers4

37

The reason you're seeing this behavior is that this within the each call is a String object instance, not a string primitive. JavaScript has both. In a switch statement, the comparison with the cases is via ===, and a string instance is not === to a string primitive.

Three ways to fix it:

  1. If you change your switch to:

    switch (String(this)) {
    

    ...that will turn it back into a primitive, whereupon your switch works.

  2. As VisioN points out in the comments below, use the arguments that $.each passes (each string — as a primitive — will be provided as the second argument):

    $.each(typ, function(index, value) {
        switch (value) {
            // ...
        }
    });
    
  3. Use any of the alternatives discussed in this other answer (one of which is a nice simple for loop).


Side note: You're falling prey to The Horror of Implicit Globals by not declaring your typ variable.

Community
  • 1
  • 1
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • 1
    Agree with VisioN. Why people use jQuery `each()` instead of a native JavaScript loop I'll never know! – hohner Feb 16 '13 at 13:28
  • 1
    @hohner: Yeah, I lean toward simple loops too, and the signature of the iteration function `$.each` requires is, um, special. :-) A lot of people like the fact that using an iteration function gives them a new scope for iteration-specific variables. That usually indicates (to me) that the function the loop is in is already too complex and needs breaking up, but that's one reason people use it. Before too long they'll be able to use `forEach` reliably for that instead (which does the rational thing of giving the entry as the *first* argument). – T.J. Crowder Feb 16 '13 at 13:32
9

jQuery is overkill here, for a correct way to do it with jQuery anyway have a look at T.J. Crowders answer.

I recommend a method that is way simpler by using a standard for-loop. It works fine:

var types = "text,password".split(",");
for (var i = 0; i < types.length; i++) {
    switch(types[i]){
        case "text":
            console.log('text');
        break;
        default: 
            console.log('default');
        break;
    }
}
TimWolla
  • 31,849
  • 8
  • 63
  • 96
  • Though TimWolla didn't find the right problem I agree that **for** is better for this purpose. I hate the way people produce unefective codes theese days. – Tomáš Zato Feb 16 '13 at 13:27
6

You use the $.each function wrongly. It should look like this:

$.each( typ, function( key, value ) {
    switch(value){

       case "text":
            console.log('text');break;
       default:
            console.log('default');break;
    }
});
Tomáš Zato
  • 50,171
  • 52
  • 268
  • 778
1

Try using switch(String(this)) instead of switch(this). And of course, initialize your variables.

Marcos
  • 4,643
  • 7
  • 33
  • 60