4
\$\begingroup\$

I have written this jQuery code to traverse the HTML markup. Basically, I am selecting data-icount attribute of each anchor tag and appending it to anchor text as (28) e.g. the ALL becomes ALL (38) in the example. 38 here is the sum of all the data-icounts under that li. Similarly, summers become summers (2) because it has two data-icount 0 and 2 so 0+2 =2.

Working example here.

I am sure there must be an improved process to do this - any comments/suggestions would be appreciated.

<script>
    $(document).ready(function(){
        var $lis = $("#tree li");   
        $lis.each(function(index) {         
            var $this = $(this);
            var $sub_uls = $this.children('ul');
            var $child_a = $this.children('a');
            var num_sub_uls = $sub_uls.length;

            var count_icount = 0;
            if(num_sub_uls > 0) {
                //var sub_lis = $this.children('a');
                var sub_lis = $sub_uls.find('a');
                //console.log('Total anchors: '+sub_lis.length);
                sub_lis.each(function(index2) {
                    count_icount += parseInt($(this).attr('data-icount'));
                    //console.log(count_icount);
                });
                //console.log("out of loop");
            } else {
                count_icount = parseInt($child_a.attr('data-icount'));
            }

            var a_txt = $child_a.text();
            $child_a.text(a_txt+' ('+count_icount+') ');
        });
    });
</script>

HTML

<ul id="tree" class="tree">
    <li class="">
        <a data-icount="0" href="/some/thing/1">ALL </a>
        <ul >
            <li class="">
                <a data-icount="7" href="/some/thing/2" >Fruits</a>
                <ul >
                    <li class="">
                        <a data-icount="5" href="/some/thing/4" >Summers</a>
                        <ul >
                            <li class=""><a data-icount="0" href="/some/thing/7">Mango</a></li>
                            <li class=""><a data-icount="2" href="/some/thing/8" >Water melon</a></li>
                        </ul>
                    </li>
                    <li class="">
                        <a data-icount="0" href="/some/thing/5" >Winters</a>
                        <ul >
                            <li class=""><a data-icount="0" href="/some/thing/9" >Tangerines</a></li>
                            <li class=""><a data-icount="0" href="/some/thing/10" >Pomegranate</a></li>
                        </ul>
                    </li>
                    <li class=""><a data-icount="1" href="/some/thing/12" >Others</a></li>
                </ul>
            </li>
            <li class="open">
                <a data-icount="0" href="/some/thing/3" class="selected">Furniture</a>
                <ul >
                    <li class=""><a data-icount="6" href="/some/thing/6" >Home</a></li>
                    <li class=""><a data-icount="0" href="/some/thing/11" >Garden</a></li>
                </ul>
            </li>
            <li class=""><a data-icount="10" href="/some/thing/13" >Games</a>
                <ul >
                    <li class=""><a data-icount="5" href="/some/thing/36" >Playstation</a></li>
                </ul>
            </li>
            <li class=""><a data-icount="2" href="/some/thing/37" >Clothes</a></li>
        </ul>
    </li>
</ul>
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Here are some tips:

1) Don't create variables if they're only used once or are a don't perform complex operations.

Old Code:

var num_sub_uls = $sub_uls.length;
var count_icount = 0;
if(num_sub_uls > 0) {

New Code:

var count_icount = 0;
if($sub_uls.length) {

2) Provide the base when using parseInt().

Old Code:

count_icount = parseInt($child_a.attr('data-icount'));

New Code:

count_icount = parseInt($child_a.attr('data-icount'), 10);

3) Eliminate commented code sections.

Old code:

var sub_lis = $sub_uls.find('a');
//console.log('Total anchors: '+sub_lis.length);
sub_lis.each(function(index2) {

New Code:

var sub_lis = $sub_uls.find('a');
sub_lis.each(function(index2) {

4) Simplify if conditions by using Truthy & Falsey .

Boolean(undefined); // => false
Boolean(null); // => false
Boolean(false); // => false
Boolean(0); // => false
Boolean(""); // => false
Boolean(NaN); // => false

Boolean(1); // => true
Boolean([1,2,3]); // => true
Boolean(function(){}); // => true

More here

Old Code:

if(num_sub_uls > 0) {

New Code:

if(num_sub_uls) {

5) Use .append() to append text to an element in jQuery.

Old Code:

var a_txt = $child_a.text();
$child_a.text(a_txt+' ('+count_icount+') ');

New Code:

$child_a.children('a').append( ' (' + count_icount + ') ' );

6) Simplify your if and else conditions.

Check if count_icount is greater than 0, instead of checking for sub_list.length;

Old Code:

if(num_sub_uls > 0) {
    var sub_lis = $sub_uls.find('a');
    sub_lis.each(function(index2) {
        count_icount += parseInt($(this).attr('data-icount'));
    });
} else {
    count_icount = parseInt($child_a.attr('data-icount'));
}

New Code:

$sub_uls.find('a').each(function(index2) {
    count_icount += parseInt($(this).attr('data-icount'));
});
if( !count_icount ){
    count_icount = parseInt($this.children('a').attr('data-icount'), 10);
}

7) Use .data() instead of .attr() to access properties that begin with data-.

.data() Documentation

Example:

<a id="thing2" data-icount="9" href="/some/thing/2" >Fruits</a>

Use

$("#thing2").data( "icount" ) == "9";

Instead of

$("#thing2").attr( "data-icount" ) == "9";

Final code:

$(function () {
    $("#tree li").each(function () {
        var $this = $(this),
        count_icount = $this.find("a").data('icount');

        $this.children('ul').find('a').each(function () {
            count_icount += parseInt($(this).data('icount'), 10);
        });
        if (!count_icount) {
            count_icount = parseInt($this.children('a').data('icount'), 10);
        }
        $this.children('a').append(' (' + count_icount + ') ');
    });
});

Demo: http://jsfiddle.net/GQkB8/2/

\$\endgroup\$
8
  • \$\begingroup\$ While I +1 this It would be good to explain a few things here. Specifically A lot of new javascripters will not understand the whole parseInt("09") issue. It would be good to include that detail here so others can understand your reasoning. \$\endgroup\$ Sep 25, 2012 at 6:35
  • \$\begingroup\$ Thanks Larry. I'll keep parseInt tip in my mind. Some of the other things like comments, and num_lis_length > 0 I kept to make code more obvious to understand but I get your point. \$\endgroup\$
    – TigerTiger
    Sep 25, 2012 at 7:30
  • \$\begingroup\$ Hi Larry - the count of very first LI's Anchor (<a data-icount="0" href="/some/thing/1">ALL </a> doesn't get added? for example, if I change it to 5 i.e. <a data-icount="5" href="/some/thing/1">ALL </a> it still reads as (38) ... any ideas how to fix this? thx \$\endgroup\$
    – TigerTiger
    Oct 3, 2012 at 16:27
  • \$\begingroup\$ In fact that is true with all other Anchors ... that is it shows count of children only ... can it be changed to include the parent count itself too? e.g. in the demo markup, Summers (2) becomes Summers (7) because Summers itself has 5 as icount. \$\endgroup\$
    – TigerTiger
    Oct 3, 2012 at 16:34
  • \$\begingroup\$ @HappyApe You found a bug with your original code then. :) \$\endgroup\$ Oct 3, 2012 at 20:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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