jQuery: Any better way to write this? Multiple each() loops

Question

var currentTallest = 0;

if($j(".eachLateDeal").exists()){
  $j(this).children().find(".resultList").each(function(i){      
    if($j(this).height() > currentTallest){
      currentTallest = $j(this).height();
    }
});

  $j(this).children().find(".resultList").each(function(i){      
    if (!$j.support.minHeight){
      $j(this).css({'height': currentTallest + 5});
    }

    $j(this).css({'min-height': currentTallest});  
});      
}

Updates: After taking all the constructive comments into consideration, I have come up with the following which seems to work as desired:

$j.fn.equalHeights = function(px) {
$j(this).each(function(){
    var currentTallest = 0;
    var results;

if($j(".eachLateDeal").length){
  results = $j(".resultList", ".eachLateDeal");
}else{
  results = $j(this).children();
}

$j.each(results, function(){
  if($j(this).height() > currentTallest){
    currentTallest = $j(this).height();
  }
});

var cssProp = {};

if (!$j.support.minHeight){
  cssProp["height"] = currentTallest + 5;
}else{
  cssProp["min-height"] = currentTallest;
}

results.css(cssProp);
});

};

Thanks all!

what are you trying to do? melt minds? or write code. please tell us what u want to do — Naftali, Mar 28 '11 at 15:28

score 1 · Answer 1 · edited May 23 '17 at 10:33

Looking at this answer you could use JavaScripts Math object to get the maximum value for you which means you can wrap all your functionality into one loop:

$(document).ready(function() {

    Array.max = function( array ){
            return Math.max.apply( Math, array );
        };

    if($j(".eachLateDeal").length) {
        var heights= new Array();
        var resultLists = $j(this).children().find(".resultList");
        resultLists.each(function() {
            heights.push($j(this).height());
        });
        var max = Array.max(heights);
        var propertyName = 'min-height';
        if(!$.support.minHeight) {
            max = max + 5;
            propertyName = 'height';
        }
        resultLists.css(propertyName, max);
    }   
});

JSFiddle example here

score 0 · Answer 2 · answered Mar 28 '11 at 15:32

0

first if you have an exists() function, it's easier if you just did:

$j(".eachLateDeal").length

it pretty much does the same thing(no need to declare an extra function)

second

$j(this).children().find(".resultList").each(function(i){

can be written as

$j(this).find(".resultList").each(function(i){

since find does look into all it's descendants

third, why do you need to go to the loop twice?

answered Mar 28 '11 at 15:32

corroded

21,406
19
83
132

i only want to set the height of those "resultList" inside the children in the placeholder, i.e 2nd level down. I had to make sure it loops every instance to get the one with the max height first then only set each of them to be of that height throughout. Any idea how not to use the 2nd loop to achieve this? – Bongbong Mar 28 '11 at 15:37

score 0 · Answer 3 · answered Mar 28 '11 at 15:33

0

One idea for your selector: use the 'context', it limits the scope of the search instead os getting all childs and find over the returned data:

var oScope = $("selectorX");
$j(".resultList", oScope).each(function(i){ ...

answered Mar 28 '11 at 15:33

Flavio CF Oliveira

5,235
13
40
63

score 0 · Answer 4 · answered Mar 28 '11 at 16:32

Using reduce you can simplify it to the following :

$.reduce = function(elems, callback, memo, arg) {
    for (var i = 0, length = elems.length; i < length; i++) {
        memo = callback(elems[i], i, memo, arg);
    }

    return memo;
};

$.fn.reduce = function(callback, memo) {
    return jQuery.reduce(this, function(elem, key, memo) {
        return callback.call(elem, elem, key, memo);
    }, memo);
};

if ($j(".eachLateDeal").length) {
    // get results
    var results = $j(this).children().find(".resultList");
    // reduce results to tallest height
    var currentTallest = results.reduce(function(val, i, memo) {
        var h = $j(this).height();
        return h > memo ? h : memo;
    }, 0);

    // set css properties
    var cssProp = {};
    if (!$j.support.minHeight) {
        cssProp["height"] = currentTallest + 5;
    } else {
        cssProp["min-height"] = currentTallest;
    }
    // set css of all elements at once.
    results.css(cssProp);
}

jQuery: Any better way to write this? Multiple each() loops

4 Answers4