patternjavascriptMinor
Traversing un-ordered list and appending attibute values to the list item
Viewed 0 times
itemthevaluestraversingattibuteappendingandlistordered
Problem
I have written this jQuery code to traverse the HTML markup. Basically, I am selecting
Working example here.
I am sure there must be an improved process to do this - any comments/suggestions would be appreciated.
HTML
```
ALL
Fruits
Summers
Mango
Water melon
Winters
Tangerines
Pomegranate
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.
$(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+') ');
});
});
HTML
```
ALL
Fruits
Summers
Mango
Water melon
Winters
Tangerines
Pomegranate
Solution
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:
New Code:
2) Provide the base when using
Old Code:
New Code:
3) Eliminate commented code sections.
Old code:
New Code:
4) Simplify if conditions by using Truthy & Falsey .
More here
Old Code:
New Code:
5) Use
Old Code:
New Code:
6) Simplify your
Check if
Old Code:
New Code:
7) Use
.data() Documentation
Example:
Use
Instead of
Final code:
Demo: http://jsfiddle.net/GQkB8/2/
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(){}); // => trueMore 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:
FruitsUse
$("#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/
Code Snippets
var num_sub_uls = $sub_uls.length;
var count_icount = 0;
if(num_sub_uls > 0) {var count_icount = 0;
if($sub_uls.length) {count_icount = parseInt($child_a.attr('data-icount'));count_icount = parseInt($child_a.attr('data-icount'), 10);var sub_lis = $sub_uls.find('a');
//console.log('Total anchors: '+sub_lis.length);
sub_lis.each(function(index2) {Context
StackExchange Code Review Q#15867, answer score: 2
Revisions (0)
No revisions yet.