1
\$\begingroup\$

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"});
        }
\$\endgroup\$
2
\$\begingroup\$

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"});
}
\$\endgroup\$
2
\$\begingroup\$

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"}
  )
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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