Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I need to find the end of a loop or the length of the object. Some context on the code:

I obtain the txt var from the DOM (that's the only way I've got by now and can't change it) and it's a formatted JSON. Here's an example of the expected JSON output:

{"SE DIO AVISO A CENTRAL":true,"OTRO":false,"SE DIO AVISO A SEGURIDAD CIUDADANA":true,"SIN ACCIÓN":false,"SE DIO AVISO A AMBULANCIA":false,"SE DIO AVISO A CARABINEROS":true,"SE DIO AVISO A BOMBEROS":false}

My idea here is to only show the ones with value trueone after the other followed by a comma, but changing that at the end of the loop for a dot. For the moment I've got the commas down but not the dot, which is why I need to know either the length of obj or the end of the $.each() loop. Any improvements on overall code are appreciated.

try{
    var obj=jQuery.parseJSON(txt);
    $(self).text(" ");
    var i = 0;
    jQuery.each(obj, function(text, val){
        if(val){
            if(i == 0){
                $(self).append(text);
            }else{
                $(self).append(", "+text);
            }
        }
        i++;
    });
}
catch(e){
}
share|improve this question
add comment

2 Answers

You can do this with jQuery.map extremely elegantly as the function allows you to do a filtered map (.map excludes any item when you return null or undefined). As $.map produces an array you can then just use Array.prototype.join and get the same output as our original code. If you want to add a period to the string after that then you can check if your string isn't empty, append a period and be on your way.

var text = $.map(obj, function(val, key) { if(val) return key; } ).join(", "); //Create an array of an objects keys where its value is truthy. Next, join the keys.
if(text !== "") text += "."; // Append a period if not an empty string

$(self).append(text);

Also you should reconsider using a try catch block around the JSON parsing as you shouldn't expect that to fail without good reason.

share|improve this answer
    
+1 for using map as a filter. –  konijn Jan 15 at 20:38
add comment

You are basically mimicking the behavior of join here. Simply collecting the correct values into an array and then joining these values will be simpler.

This is what I would counter-propose:

try{
  var o = jQuery.parseJSON( s ),
      keys = [];
  jQuery.each(o, function(key, value){
    if(value){
      keys.push(key);
    }
  }
  $(self).text( " " + keys.join(",") + "." );
}
catch(e){
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.