/    Sign up×
Community /Pin to ProfileBookmark

javascript function

I have a javascript function. This takes long time to execute. How do I make it faster without loosing the functionality.

file attached

[upl-file uuid=cfd23195-3275-4e24-86ce-0beaf068a2e8 size=1kB]src.txt[/upl-file]

to post a comment
JavaScript

7 Comments(s)

Copy linkTweet thisAlerts:
@justinbarneskinJun 15.2010 — Define variables ahead of the function if they are static.
Copy linkTweet thisAlerts:
@cofactorauthorJun 15.2010 — Define variables ahead of the function if they are static.[/QUOTE] did not get you . How does that reduce time complexity ? I don't think that will solve the problem . Its not the problem with static but its probabily the problem with two for loops running to find a match.....I don't know other way to find the match other than this.

anyway, Could you please clarify your ideas further ?
Copy linkTweet thisAlerts:
@MrRedJun 15.2010 — if you know the priority/probability of occurrence of the various data could you divide the if statement into three individual tests based on the likelihood of each test being appropriate?
if((terrId1 == terrId2)&&(hdnIsApproveEnabled=='8')&&(hdnclsfname==clsfname))

does using the identity/congruent test help with numbers? Maybe not but it doesn't take long to try.
eg hdnIsApproveEnabled===8
would removing the parentheses help a bit?

if(terrId1 == terrId2&&hdnIsApproveEnabled=='8'&&hdnclsfname==clsfname)

I would have thought instantiating the variables outside the loops (at the start of the function) and assigning only inside the loops would help a little.

ANDdocument.forms[0].does not alter inside the loops, I would have assigned a variable to it at the head and used that in loops to get at attributes.

andfor(var m=0; m<terrScheduleIdHiddenLength; m++) works faster as for(var m=terrScheduleIdHiddenLengt-1; m>-1; m--) providing you can tolerate the direction, because it doesn't have to read the value to test. (literals and all that).

You may find a lot of tweaks instead of one magic move will be the solution.
Copy linkTweet thisAlerts:
@cofactorauthorJun 16.2010 — Thanks MrRed.

I have read your comments . I tried your comments. But this does not improve the situation . script is still slow.

Do you have any Jquery solution to try this ?
Copy linkTweet thisAlerts:
@Declan1991Jun 16.2010 — To have any useful input, we would have to know what your aims are.
Copy linkTweet thisAlerts:
@SparoHawkJun 16.2010 — I don't see any way to reduce code. Maybe knowing how everything works and what you are trying to validate may help more.

All I could do was remove your references to document.forms[0] to a variable and skip your first statement to see if an element exists in the form:

[CODE]function checkAccept()
{
// Here, create a reference to the forms[0]
var f = document.forms[0];

// Here, you can put the reference in the if. If it does not exist it will return undefined, which = false
if(f.terrScheduleIdHidden)
{
var terrScheduleIdHiddenLength = f.terrScheduleIdHidden.length;

if(terrScheduleIdHiddenLength > 0)
{
for(var m = 0; m < terrScheduleIdHiddenLength; m++)
{
var terrId1 = f.terrScheduleIdHidden[m].value;
var clsfname = f.classificationName[m].value;
var hdnTerritoryScheduleidLength = f.hdnTerritoryScheduleid.length;

for(var n = 0; n < hdnTerritoryScheduleidLength; n++)
{
var terrId2 = f.hdnTerritoryScheduleid[n].value;
var hdnclsfname = f.hdnClassificationName[n].value;
var hdnIsApproveEnabled = f.hdnIsApproveEnabled[n].value;

if(terrId1 == terrId2 && hdnIsApproveEnabled == '8' && hdnclsfname == clsfname)
{
//callsomefunction(m, n);
break;
}
}
}
}
}
}[/CODE]
Copy linkTweet thisAlerts:
@acestuffJun 16.2010 — There are loads of variables that are getting set that just aren't needed. For example:

var terrScheduleIdHiddenExist = (document.forms[0].terrScheduleIdHidden)?true:false;

if(terrScheduleIdHiddenExist)
[/QUOTE]

With this, you're performing one if statment to set the variable then another of that variable - why not just do one? Not a massive time saver but better.


The main time cost is them loops which I'm guessing are quite long.

It's the 2nd loop where I think you're losing on. You're setting them 3 variables (terrId2, hdnclsfname, hdnIsApproveEnabled) and then performing that if statement with 3 conditions. What happens if the if return false on the first statement - you've set the next two variables (hdnIsApproveEnabled, hdnclsfname) for no reason as they're not going to get used!


The final thing that comes to mind is this:
[CODE]document.forms[0][/CODE]
This keeps using the DOM (might be wrong). Why not just store this as a variable (my_form, etc) then refer to this in your code.


I would personally use the below:
[CODE]
function checkAccept() {
var my_form = document.forms[0];
if(my_form.terrScheduleIdHidden) {

for(var m = 0; m < my_form.terrScheduleIdHidden.length; m++) {
var terrId1 = my_form.terrScheduleIdHidden[m].value;
var clsfname = my_form.classificationName[m].value;

for(var n = 0; n < my_form.hdnTerritoryScheduleid.length; n++) {
var hdnclsfname = my_form.hdnClassificationName[n].value;

if (hdnclsfname == clsfname) {
var terrId2 = my_form.hdnTerritoryScheduleid[n].value;

if (terrId1 == terrId2) {
var hdnIsApproveEnabled = my_form.hdnIsApproveEnabled[n].value;

if (hdnIsApproveEnabled == "8") {
//callsomefunction(m, n);
break;
}
}
}
}
}
}
}
[/CODE]
×

Success!

Help @cofactor spread the word by sharing this article on Twitter...

Tweet This
Sign in
Forgot password?
Sign in with TwitchSign in with GithubCreate Account
about: ({
version: 0.1.9 BETA 5.21,
whats_new: community page,
up_next: more Davinci•003 tasks,
coming_soon: events calendar,
social: @webDeveloperHQ
});

legal: ({
terms: of use,
privacy: policy
});
changelog: (
version: 0.1.9,
notes: added community page

version: 0.1.8,
notes: added Davinci•003

version: 0.1.7,
notes: upvote answers to bounties

version: 0.1.6,
notes: article editor refresh
)...
recent_tips: (
tipper: @AriseFacilitySolutions09,
tipped: article
amount: 1000 SATS,

tipper: @Yussuf4331,
tipped: article
amount: 1000 SATS,

tipper: @darkwebsites540,
tipped: article
amount: 10 SATS,
)...