Those are some great optimization tips, Josh. I can see a couple of other things that could be optimized, but it's too hard to comment on them with the code torn up on the shop floor. :-) So Nizam, how about incorporating Josh's suggestions into a new revision of the code, and then we all can take a second look at it.
But Matt is right: this is the wrong group. jquery-dev is for discussion of the internals of jQuery itself. Let's move the discussion over to the main jquery group. See you there! -Mike On Sun, Nov 22, 2009 at 8:41 PM, seasoup <[email protected]> wrote: > > Hi, > > In the below Code takes too much of time for execute I don't know > > did I made any mistake. > > Happy to help, I love optimizing code. > > > > > function chkLicense() { > > var firstLicDD = '', secLicDD = '', ddLicValue = '', ddStateValue > > = ''; > > var firstStateDD = '', SecStateDD = ''; > > var IsTwoDentLICandSTATEsame = false, firstDDIndex = null, > > isLicenseNotSel = false; > > var isStateNotSel = false, isTwoLICsameExceptDental = false; > > var isDntlMedHaveAllState = false; isLicEmpty = false; > > var $myTable = $('#Licenses tr'); > > > $myTable.each(function(index) { > .each is slower then a for loop. use > > for (var a = 0, len = $('#Licenses tr').size(); a < len; a++) { > > storing the length in a variable is faster because the length of the > array doesn't need to be accessed in every loop. > > var LicNo = '', ddindex = ''; > > firstDDIndex = index; > > if (index > 0) { > > firstLicDD = $(this).find('td :DropDownList').eq(2).val(); > > Store $(this) in a variable for reuse, putting this in the jQuery > function is a costly process. > > $this = $(this); > firstLicDD = $(this).find('td :DropDownList').eq(2).val(); > > > /*For PA DropDown*/ > > //firstStateDD = $(this).find('td > :DropDownList').eq(11).val(); > > /*For Html DropDown*/ > > firstStateDD = $(this).find('td :DropDownList').eq(10).val > > (); > > even better, store $(this).find('td :DropDownList') in a variable > var ddl = $(this).find('td :DropDownList').; > firstStateDD = ddl.eq(10).val(); > > > //if ((firstLicDD == 1) || (firstLicDD == 5)) { > > if (({ 1: 1, 5: 1 })[firstLicDD]) { > > if (firstStateDD == 0) { > > isDntlMedHaveAllState = true; > > } > > } > > //if (firstLicDD == '' || firstLicDD == null) { > > if (firstLicDD == '' || firstLicDD == null) { > > isLicenseNotSel = true; > > } > > if (firstStateDD == '' || firstStateDD == null) { > > isStateNotSel = true; > > } > > LicNo = $(this).find("td").eq(2)[0].childNodes[0].value; > > use the stored variable and you can just get the second element > directly without using .eq(2) > $this.find('td')[2].childNodes[0].value; > > > > if (LicNo == '' || LicNo == null) { > > isLicEmpty = true; > > } > > > > if (firstLicDD != "") { > > $myTable.each(function(index) { > > if (index > 0) { > > secLicDD = '', SecStateDD = ''; > > secLicDD = $(this).find('td > :DropDownList').eq(2).val(); > > use the stored variable > secLicDD = ddl.eq(2).val(); > > > /*For PA DD*/ > > //SecStateDD = $(this).find('td > :DropDownList').eq(11).val(); > > use the stored variable > SecStateDD = ddl.eq(11).val(); > > > /*For Html.DD*/ > > SecStateDD = $(this).find('td > :DropDownList').eq(10).val(); > > use the stored variable > SecStateDD = ddl.eq(10).val(); > > > if (SecStateDD != '') { > > for (var i = 0; i <= parseInt(SecStateDD) + > 1; i++) { > > this will run parseInt in every iteration of the for loop, instead > store it in a variable > for (var i = 0, len = parseInt(SecStateDD) + 1; i <= len; i++) { > > > /*For PA DD*/ > > //if ($(this).find('td > :DropDownList')[11][i].value == SecStateDD) { > > use the stored variable > ddl[11][i].value == SecStateDD) { > > /*For Html.DD*/ > > if ($(this).find('td > :DropDownList')[10][i].value == SecStateDD) { > > use the stored variable. > ddl[10][i].value == SecStateDD) { > > > ddindex = i; > > } > > } > > } > > if (secLicDD != "") { > > if (firstDDIndex != index) { > > if (firstLicDD == secLicDD) { > > if ((firstLicDD == 1) && (secLicDD == > 1)) { > > if ((secLicDD == 1) && > (SecStateDD > 0)) { > > /*For PA DD*/ > > //var ddSelectedText = > $(this).find('td :DropDownList')[11][ddindex].innerHTML; > > use the stored variable > var ddSelectedText = ddl[11][ddindex].innerHTML; > > > /*For Html DD*/ > > var ddSelectedText = > $(this).find('td :DropDownList')[10][ddindex].innerHTML; > > use stored variable > var ddSelectedText = ddl[10][ddindex].innerHTML; > > > var Prvid = > $('#PrimaryKey')[0].value; > > var Prvid = $('#PrimaryKey').val(); > > > var stateCode = > ddSelectedText; > > var LicenseTypeID = > firstLicDD; > > > //fnIsAllStateForDenMed(Prvid, stateCode, LicenseTypeID); > > } > > if (SecStateDD != "") { > > if (firstStateDD == > SecStateDD) { > > > > IsTwoDentLICandSTATEsame = true; > > } > > } > > } > > else { > > isTwoLICsameExceptDental = true; > > } > > } > > } > > } > > } > > }); > > > > } > > } > > }); > > if (isStateNotSel || isLicenseNotSel || IsTwoDentLICandSTATEsame > > || isTwoLICsameExceptDental || isLicEmpty || > > isDntlMedHaveAllState) { > > var message = ' '; > > if (isLicenseNotSel) { > > message += " Select Licence \n" + "<br/>"; > > concatenating strings has a cost, try and avoid extra usages when > possible. > message += " Select Licence \n<br/>"; > > > } > > if (isStateNotSel) { > > message += " Select State" + "<br/>"; > > same > message += " Select State<br/>"; > > > } > > if (isDntlMedHaveAllState) { > > message += "Dental and Medi-Caid License Type should not > > have All State" + "<br/>"; > > same > message += "Dental and Medi-Caid License Type should not have All > State<br/>"; > > > } > > if (isTwoLICsameExceptDental) { > > message += "Two License Types should not be Same Except > > Dental" + "<br/>"; > > same > > > } > > if (IsTwoDentLICandSTATEsame) { > > message += "Dental License Types should have unique state" > > + "<br/>"; > > } > > if (isLicEmpty) { > > message += "Enter License Number" + "<br/>"; > > } > > $('#LicenseErrormsg').html(message); > > I didn't see if this has a lot of nodes in it, but if it does this can > be slow. jQuery checks every node before overwriting it for events to > make sure that no circular references exist because there it could > cause a memory leak in ie. If you know there are no events on these > nodes it is much quicker to do: > > $('#LicenseErrormsg')[0].innerHTML = message; > > > return false; > > } > > else { > > $('#LicenseErrormsg').html(""); > > same as above. > $('#LicenseErrormsg')[0].innerHTML = ''; > > > return true; > > } > > > > } > > > > Thank You in Advance. > > Nizam > > > Hope this helps, > Josh Powell > > -- > > You received this message because you are subscribed to the Google Groups > "jQuery Development" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<jquery-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/jquery-dev?hl=. > > > -- You received this message because you are subscribed to the Google Groups "jQuery Development" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/jquery-dev?hl=.
