current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite

In this code, I'm getting the json from backend. Once I find the json availability, I loop and make 5 columns. Once the columns are made, I append after the legend of my form.

Can this be improved any further?

if(data.hasOwnProperty("allLocales")){

            var col0 = $("<fieldset />"), 
                col1 = $("<fieldset />"),
                col2 = $("<fieldset />"),
                col3 = $("<fieldset />"),
                col4 = $("<fieldset />");

            $.map(data["allLocales"], function(value, i){

                var name = value.name;

                if(i % 5 === 0 ){
                    col0.append($("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)));
                }else if(i % 5 === 1){
                    col1.append($("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)));
                }else if(i % 5 === 2){
                    col2.append($("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)));
                }else if(i % 5 === 3){
                    col3.append($("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)));
                }else if(i % 5 === 4){
                    col4.append($("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)));
                }

            })

            $("form legend").after(col0,col1,col2,col3,col4).end().find("span.submit").css({display:"block"});
        }
share|improve this question
add comment

2 Answers

up vote 2 down vote accepted

Rather use an array. This allows you to write the following:

if (data.hasOwnProperty("allLocales")) {
    var col = [];
    for(var i = 0; i < 5; i++) {
        col[i] = $("<fieldset />");
    }
    $.map(data["allLocales"], function (value, i) {
        var name = value.name;
        $col[i % 5].append($("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)));
    })
    $("form legend").after(col).end().find("span.submit").css({display: "block"});
}
share|improve this answer
 
Fantastic.. very thanks to you. –  3gwebtrain Jul 18 '13 at 12:44
add comment
up vote 2 down vote

First you should not create on function for each column. In place of :

var col0 = $("<fieldset />"), 
    col1 = $("<fieldset />"),
    col2 = $("<fieldset />"),
    col3 = $("<fieldset />"),
    col4 = $("<fieldset />");

You should use :

var col_type  = "<fieldset />";


var cols      = {}

And then add it to your JSON :

cols["col"+ i % 5]  = $(col_type)...

So, you'll get a JSON object of this form :

{
  "col0":   <YOUR_jQuery element>,
  "col1":   <YOUR_jQuery element>,
  "col2":   <YOUR_jQuery element>,
  "col3":   <YOUR_jQuery element>,
  "col4":   <YOUR_jQuery element>
}

JSON are not support in jQuery for after() iteration. You must use an Array.

cols.push($(col_type)...)

And you won't need one variable for each new column.

Could you give a sample of your data.allLocales ? And a sample of your HTML code ?

And no if(data.hasOwnProperty("allLocales")) is needed

Described :

for( column in data["allLocales"] ) {
  var name = data["allLocales"][column].name

  $("form legend").after(
    $("<fieldset />")
    .append(
      $("<label />")
      .text(name)
      .prepend(
        $("<input type='checkbox' />")
        .attr("value", name)
      )
    )
  )
 .end()
 .find("span.submit")
 .css({display: "block"});
}

Shorter :

for( column in data["allLocales"] ) {
  var name = data["allLocales"][column].name
  $("form legend").after($("<fieldset />").append(
    $("<label />").text(name).prepend($("<input type='checkbox' />").attr("value", name)))).end().find("span.submit").css({display: "block"}
  )
}
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.