Tell me more ×
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

I'm trying to get rid of some of the horrible nested loops in the following code. Essentially is moving TDs to top rows based on whether the current row has the current cell. You can see the execution here: http://jsfiddle.net/Jq8sk/

How can I clean it up and avoid this level of depth. Any help is appreciated.

var trs = $('.trs');
var arr = ['n0', 'n1', 'n2'];
for (var i = 0; i < trs.length; i++) {
    for (var j = 0; j < arr.length; j++) {
        var tds = trs[i].getElementsByClassName('tds ' + arr[j]);
        if (tds.length == 0) {
           for (var x = i + 1; x < trs.length; x++) {
               var tds_offset = trs[x].getElementsByClassName('tds ' + arr[j]);
               if (tds_offset.length > 0) {
                  trs[i].appendChild(tds_offset[0]);
                  break;
               }
           }
        }
    }
}
share|improve this question
1  
Out of curiosity, if you're using jquery, why only use it on the first line? – James Montagne Feb 24 '12 at 21:32
the whole application uses jquery, but in that part of the code, only the first line... – Mike McGrath Feb 24 '12 at 21:44
thanks everybody! – Mike McGrath Feb 24 '12 at 23:08

2 Answers

up vote 0 down vote accepted

This may not actually be any better as far as performance goes, but I believe it is equivalent to your code (and possibly more readable?).

http://jsfiddle.net/Jq8sk/1/

var arr = ['n0', 'n1', 'n2'];

$('.trs').each(function(){
   var $row = $(this);

    for(var i=0;i<arr.length;i++){

        if($row.children(".tds." + arr[i]).length === 0){
            // td doesn't exist, see if any later rows have it

            var $tds = $row.nextAll(".trs").children(".tds." + arr[i]);

            if($tds.length > 0){
               $row.append($tds[0]);   
            }
        }
    } 
});
share|improve this answer
thanks, that should be better. – Mike McGrath Feb 24 '12 at 22:30
up vote 0 down vote

Consider this:

$( '.trs' ).each( function ( i, row ) {
    $.each( arr, function ( i, className ) {
        if ( !$( row ).children().hasClass( className) ) {
            $( row ).nextAll().children( '.' + className+ ':first' ).appendTo( row );
        }
    });
});

Live demo: http://jsfiddle.net/FUdpE/1/

share|improve this answer

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.