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

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
add comment

2 Answers 2

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
add comment
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
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.