• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 576
  • Last Modified:

JavaScript slowing down IE??

Situation:

I've got a problem and don't know if there is a solution.  I've got a rather long html page that simply won't fit on this page.  You can see it by going to http://evalplus.webasics.net/javascriptissue.html.

If you are using IE, the page will likely take forever to load. It loads a ton faster in FF.  I'm going to post the javascript code here that is slowing the page down.    The following functions are being called when the page loads: "onload="dummy('B'); dummy('C'); dummy('D'); totalWeighted(); hideComments(evaluation);"

I think the functions I'm having the most trouble with is the hideComments(); and the showLayer();, but I'm not entirely sure anymore.  Everything runs just fine if I'm working with about 50 or less of these radio button groups, but this one has over 200.  

Questions:

Are there just too many questions (radio button groups) for javascript to handle this?
Is the javascript the problem (meaning poorly written) or is it something else, like HTML?
If it is poorly written javascript, what needs to be fixed?


Start Javascript code section:

<script language="JavaScript" type="text/javascript">


function dummy(prefix) {
 var count = 0;
 var sum = 0;
 for (xx=0; xx < document.evaluation.elements.length; xx++)
 {
  if (document.evaluation.elements[xx].type == 'radio') {
   if (document.evaluation.elements[xx].checked == true) {
    var bName = document.evaluation.elements[xx].name;
      if (bName.indexOf(prefix) == 0) {
     count++;
     sum = eval(document.evaluation.elements[xx].value) + eval(sum);
    }
   }
  }
 }
 if (sum > 0 && count > 0) {
  var avg = Math.round (eval(sum / count)*100)/100;
 
  if (prefix == 'B')
  document.evaluation.baverage.value = avg;
 
  if (prefix == 'C')
  document.evaluation.caverage.value = avg;
 
  if (prefix == 'D')
  document.evaluation.daverage.value = avg;
 
  //Add more prefixes here...
 }
 
 if (prefix == 'B') {
 var bweighted =  Math.round (eval (sum / count * .50)*100)/100;
 if(isNaN(bweighted)){
document.evaluation.bweighted.value = 0;
return false;
}
 document.evaluation.bweighted.value = bweighted;}
 
 if (prefix == 'C'){
 var cweighted = Math.round (eval (sum / count * .35)*100)/100;
if(isNaN(cweighted)){
document.evaluation.cweighted.value = 0;
return false;
}
 document.evaluation.cweighted.value = cweighted;}
 
 if (prefix == 'D'){
 var dweighted =  Math.round (eval(sum / count * .15)*100)/100;
 if(isNaN(dweighted)){
document.evaluation.dweighted.value = 0;
return false;
}
 document.evaluation.dweighted.value = dweighted;}
 
var aweighted = parseFloat (document.evaluation.bweighted.value) + parseFloat (document.evaluation.cweighted.value) + parseFloat (document.evaluation.dweighted.value);
 document.evaluation.allweighted.value =  Math.round (aweighted *100)/100;
 
}


function totalWeighted(){
var totalweighted = parseFloat (document.evaluation.bweighted.value) + parseFloat (document.evaluation.cweighted.value) + parseFloat (document.evaluation.dweighted.value);
 document.evaluation.allweighted.value =  Math.round (totalweighted *100)/100;
}


 
function archive() {

 var archiveit = window.confirm("This action will complete and finalize this evaluation.  No further changes will be allowed.  Click OK to continue.");

if (archiveit) {
document.evaluation.action='index.cfm?fuseaction=eval.ArchiveCurrentYear&hash=f3ebbbe8394f68dab52a59dbfd3b69dc';
document.evaluation.submit()
   }
}

function finish() {

 var finishit = window.confirm("This action will finish the evaluation for review with the employee. It also locks in the Employee's current Job Number and Department Number.  Do not continue if these 2 items are not correct.  Click OK if you wish to continue.");

if (finishit) {
document.evaluation.action="index.cfm?fuseaction=eval.FinishCurrentYear&hash=ea6c80bcda3287f668c64e39b0adead8";
document.evaluation.submit()
   }
}

function validateComments(obj){
    var flag = false;
     var rdgroup = '';
     var rdgroup_array = new Array();

    for(var i=0;i<obj.length;i++){
       if(obj.elements[i].type=='radio'){
         if(rdgroup!=obj.elements[i].name){
              rdgroup_array[rdgroup_array.length] = obj.elements[i].name;
          }
       }
     }    

     for(var i=0;i<rdgroup_array.length;i++){
       var objrdgroup = document.forms[obj.name][rdgroup_array[i]];
       var objrdgroupval = getCheckedValue(objrdgroup);
       var objrdcomment = eval('obj.' + rdgroup_array[i]+'comment');
       if(objrdgroupval!=''){
         if(parseInt(objrdgroupval)==1 && objrdcomment.value==''){
            alert('Scores of 1 need a comment!');
          objrdcomment.focus();
            return false;
    }
   
   
   
     if(parseInt(objrdgroupval)==5 && objrdcomment.value==''){
            alert('Scores of 5 need a comment!');
          objrdcomment.focus();
            return false;
    }
       }
       else{
         alert('You have not answered all the questions on this evaluation.  Please review and complete the unanswered questions.');
   
          return false;
       }
     }
     finish();
  }
 
  function validateCommentsArchive(obj){
    var flag = false;
     var rdgroup = '';
     var rdgroup_array = new Array();

    for(var i=0;i<obj.length;i++){
       if(obj.elements[i].type=='radio'){
         if(rdgroup!=obj.elements[i].name){
              rdgroup_array[rdgroup_array.length] = obj.elements[i].name;
          }
       }
     }    

     for(var i=0;i<rdgroup_array.length;i++){
       var objrdgroup = document.forms[obj.name][rdgroup_array[i]];
       var objrdgroupval = getCheckedValue(objrdgroup);
       var objrdcomment = eval('obj.' + rdgroup_array[i]+'comment');
       if(objrdgroupval!=''){
         if(parseInt(objrdgroupval)==1 && objrdcomment.value==''){
            alert('Scores of 1 need a comment!');
          objrdcomment.focus();
            return false;
    }
   
   
   
     if(parseInt(objrdgroupval)==5 && objrdcomment.value==''){
            alert('Scores of 5 need a comment!');
          objrdcomment.focus();
            return false;
    }
       }
       else{
         alert('You have not answered all the questions on this evaluation.  Please review and complete the unanswered questions.');
   
          return false;
       }
     }
     archive();
  }

 
 
 
  function getCheckedValue(objrdgroup){
    for(var i=0;i<objrdgroup.length;i++){
       if(objrdgroup[i].checked){
         return objrdgroup[i].value;
       }
     }
    return '';
  }


 
 
function showLayer(theRadio){
  theCell = theRadio.form[theRadio.name+"comment"].parentNode;
 theWord = theRadio.form["Word" + theRadio.name]
  if(theRadio.value==0){
    theCell.style.display = "none";
  }
 
  else if(theRadio.value==1){
    theCell.style.display = "block";
  theWord.value = "Action Plan:";
  }
 
  else if(theRadio.value==2){
    theCell.style.display = "block";
 theWord.value = "Action Plan:";
  }
 
 
 else if(theRadio.value==4){
    theCell.style.display = "block";
 theWord.value = "Comment:";
  }
 
  else if(theRadio.value==5){
    theCell.style.display = "block";
 theWord.value = "Comment:";
  }
  else {
    theCell.style.display = "none";
  }
}



function hideComments(obj){
    var flag = false;
     var rdgroup = '';
     var rdgroup_array = new Array();

    for(var i=0;i<obj.length;i++){
       if(obj.elements[i].type=='radio'){
         if(rdgroup!=obj.elements[i].name){
              rdgroup_array[rdgroup_array.length] = obj.elements[i].name;
          }
       }
     }    

     for(var i=0;i<rdgroup_array.length;i++){
       var objrdgroup = document.forms[obj.name][rdgroup_array[i]];
       var objrdgroupval = getCheckedValue(objrdgroup);
       var objrdcomment = eval('obj.' + rdgroup_array[i]+'comment');
  var objrdword = eval('obj.' + 'Word' + rdgroup_array[i]);
    if(objrdgroupval==''){
objrdcomment.parentNode.style.display = "none";
       }
 
    else if(objrdgroupval== 1){
objrdcomment.parentNode.style.display = "block";
objrdword.value = "Action Plan:";
       }
   
    else if(objrdgroupval== 2){
objrdcomment.parentNode.style.display = "block";
objrdword.value = "Action Plan:";
       }
   
   
    else if(objrdgroupval== 4){
objrdcomment.parentNode.style.display = "block";
objrdword.value = "Comment:";
       }
   
    else if(objrdgroupval== 5){
objrdcomment.parentNode.style.display = "block";
objrdword.value = "Comment:";
       }
    else {objrdcomment.parentNode.style.display = "none";
    }
     }
   }

 


</script>
0
LManesse
Asked:
LManesse
  • 9
  • 7
  • 5
  • +3
1 Solution
 
virmaiorCommented:
looking at your code, the reason why it runs slowly is obvious.

"onload="dummy('B'); dummy('C'); dummy('D'); totalWeighted(); hideComments(evaluation);"

look at your dummy function

change your onload to this:
"onload="dummy(['B','C','D']); totalWeighted(); hideComments(evaluation);"

change this part of your dummy function:
 for (xx=0; xx < document.evaluation.elements.length; xx++)
 {
  if (document.evaluation.elements[xx].type == 'radio') {
   if (document.evaluation.elements[xx].checked == true) {
    var bName = document.evaluation.elements[xx].name;
     curprefix = bName[0];
     if (prefix.indexOf(curprefix) > - 1) {
     count[curprefix] = count[curprefix] + 1;
     sum[curprefix] = int(document.evaluation.elements[xx].value) + int(sum[curprefix])w;
    // sum[curprefix] = eval(document.evaluation.elements[xx].value) + eval(sum[curprefix])w;
    //do you really need eval?
    //http://www.mozilla.org/js/language/js20-1999-02-18/types.html
    }
   }
  }
 }

... the problem with your current method is that you're looping through the elements superfluously...
btw, there may be syntax errors in what I gave you.  but it should be about faster by a factor of the number of prefixes
(e.g. faster by nearly 4x for 4 prefixes, faster by probably 25x for the entire alphabet).
0
 
virmaiorCommented:
the reason FF is faster is just that its JS engine is that much faster.
0
 
LManesseAuthor Commented:
Thanks! I've now gained about 20 seconds between the two (58 seconds vs. 118 seconds).  The old version is now posted as http://evaldev.webasics.net/javascriptissue_original.html  and the new version is http://evaldev.webasics.net/javascriptissue.html .  

I'm wondering about the showLayer() and hideCommments() functions.  You'll notice that if you click on one of the radio groups, it takes about 17 seconds to show/hide the textarea.  It just seems incredibly long.  Any thoughts there???

0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
Michel PlungjanIT ExpertCommented:
Neither page is available
0
 
virmaiorCommented:
to be honest, I would probably break the loop that i accelerated even out of the function it was in

have it populate an array that just contains the radio elements, then scan this array in the dummy function and the other two.

alternately do all of your checking inside of this loop no matter what your purpose is (whether it's for dummy or the alternate functions)

the slow down is the same in cause... in your showcomments() and hidecomments()

it would be wise to keep a reduced list of just those elements that are radio buttons to speed these functions up
or ideally to have a one to one correspondence between your textareas (or a div containing all of the relevant text areas) and radio buttons.  and then name them in such a way that by knowing the name of one, you know what to show and hide in the other.

the page link you gave isn't working.
0
 
Michel PlungjanIT ExpertCommented:
And yes, the script is extremely poorly written with code repeating itself and looping over and over.
0
 
LManesseAuthor Commented:
sorry about the link mistakes.  Here are the right links:

http://evalplus.webasics.net/javascriptissue_original.html  (original)
http://evalplus.webasics.net/javascriptissue.html (with your newer javascript)

I need to make sure you know I'm a novice with javascript, so I'm vaguely grasping (break the loop & populate an array?) what you are saying virmajor.  Any example to work off would be great.  
0
 
Michel PlungjanIT ExpertCommented:
Create a frameset and split the forms in 4 or 5 pages.
Have teh validation in the top frameset page and use
top.functionName

and save the results in the top too.
that will make the pages less massive and faster
0
 
LManesseAuthor Commented:
the other thing I've not mentioned is this page I'm showing is the HTML result from a ColdFusion page, so the page is dynamic.  The number of questions on this page are the most (over 200) I've ever seen.  Most of my clients keep the questions to a max of about 50 so I haven't worried about this page... until now.
0
 
virmaiorCommented:
mplungjan - i don't think that would really work (I haven't looked at the page yet)

I'll show you my loop solution either tomorrow (or monday), I've got something like it at work (for a different purpose).
0
 
Michel PlungjanIT ExpertCommented:
??? Why would that not work???
0
 
virmaiorCommented:
mplungjan: because i don't think his main slow downs are from the page loading time.  That and it's probably a major pain to rewrite the CF to use a frame solution.
since it's the javascript that's killing the load times, the key is in finding better ways to do the JS.

take the modified function I gave you,
 remove the modified loop.

add to the beginning of your onLoad the following: var myradios = slurpy();

here is slurpy:

function slurpy()
{
for (xx=0; xx < document.evaluation.elements.length; xx++)
 {
  if (document.evaluation.elements[xx].type == 'radio') {
  slurps++;
  slurpRadios[slurps] = document.elements[xx];
   }
  }
  //alert("total elements:" + xx);
  //alert("total radios:" + slurps);
  return slurpRadios;
 }

now your loop in dummy looks like this:
for (xx=0; xx < myradios.length; xx++)
{
   if (myradios[xx].checked == true) {
    var bName = myradios[xx].name;
     curprefix = bName[0];
     if (prefix.indexOf(curprefix) > - 1) {
     count[curprefix] = count[curprefix] + 1;
     sum[curprefix] = int(myradios[xx].value) + int(sum[curprefix])w;
    }
}

basically anywhere you were using the loop through all of the elements, now you just scan myradios
which gets populated as the first thing when the page is loaded.
my guess is there are far fewer radios than there are total elements, so this should speed things up
to check you could uncomment the alerts;
0
 
Michel PlungjanIT ExpertCommented:
You probably want to add and change something
Not very thoughtful code virmaior, please be less sloppy.

myRadios=null;
function countRadios() {
  myRadios new Array()
  var cnt=0; // can use myRadios.length each time but that is also costly
  var elems=document.evaluation.elements;
  for (var i=0, n=elems.length;i < n; i++) {
    if (elems[i].type == 'radio') {
      myRadios[cnt++] = elems[i];
    }
  }
}

now
at load time do a countRadios()
and then you can use (not tested but cleaned up

for (i=0, n=myRadios.length; i < n; i++) {
   if (myRadios[i].checked) {
     var bName = myRadios[i].name;
     curPrefix = bName[0];
     if (prefix.indexOf(curPrefix) > - 1) {
      count[curPrefix] = count[curPrefix] + 1;
      sum[curPrefix] = parseInt(myRadios[i].value) + parseInt(sum[curPrefix])); // not tested and not understood what )w was
    }
  }
}
0
 
davidlars99Commented:
please whatever the case is IE's javascript is much faster than FF's and any other
0
 
davidlars99Commented:
let's say even more stable than FF's
0
 
Michel PlungjanIT ExpertCommented:
Noise? Two useful comment?
0
 
davidlars99Commented:
what do you mean..?
0
 
searlasCommented:
Changed hideComments and dummy functions... Now runs a LOT quicker:

function dummy(prefix) {
  var prefixes = (prefix instanceof Array) ? prefix : new Array(prefix);
  var count = { "B": 0, "C": 0, "D": 0 };
  var sum = { "B": 0, "C": 0, "D": 0 };
  var elements = document.evaluation.elements;
  var len = elements.length;
  for (xx = 0; xx < len; xx++) {
    var current = elements[xx]
      if (current.type == 'radio') {
      if (current.checked == true) {
        var name = current.name;
        for (pp in prefixes) {
          var prefix = prefixes[pp];
          if (name.indexOf(prefix) == 0) {
            count[prefix]++;
            sum[prefix] += parseInt(current.value, 10);
          }
        }
      }
      }
  }



  for (pp in prefixes) {
    var prefix = prefixes[pp];
    if (sum[prefix] > 0 && count[prefix] > 0) {
      var avg = Math.round ((sum[prefix] / count[prefix])*100)/100;
      if (prefix == 'B')
      document.evaluation.baverage.value = avg;

      if (prefix == 'C')
      document.evaluation.caverage.value = avg;

      if (prefix == 'D')
      document.evaluation.daverage.value = avg;

      //Add more prefixes here...
    }

    if (prefix == 'B') {
      var bweighted =  Math.round ( (sum[prefix] / count[prefix] * .50)*100)/100;
      if(isNaN(bweighted)) {
      document.evaluation.bweighted.value = 0;
      return false;
      }
      document.evaluation.bweighted.value = bweighted;
    }

    if (prefix == 'C') {
      var cweighted = Math.round (eval (sum[prefix] / count[prefix] * .35)*100)/100;
      if(isNaN(cweighted)){
      document.evaluation.cweighted.value = 0;
      return false;
      }
      document.evaluation.cweighted.value = cweighted;
    } else if (prefix == 'D') {
      var dweighted =  Math.round (eval(sum[prefix] / count[prefix] * .15)*100)/100;
      if(isNaN(dweighted)){
      document.evaluation.dweighted.value = 0;
      return false;
      }
      document.evaluation.dweighted.value = dweighted;
    }

    var aweighted = parseFloat (document.evaluation.bweighted.value) + parseFloat (document.evaluation.cweighted.value) + parseFloat (document.evaluation.dweighted.value);
    document.evaluation.allweighted.value =  Math.round (aweighted *100)/100;      
  }

}


function hideComments(form){
  var flag = false;
  var rdgroup = '';
  var rdgroup_array = new Array();
  var comments = {};
  var textareas = {};

  var elements = form.elements;
  var length = elements.length;
  for(var i=0;i<length;i++){
    var current = elements[i];
    var name = current.name;
    if(current.type=='radio'){
      if(rdgroup != name){
      rdgroup_array[rdgroup_array.length] = name;
      rdgroup = name;
      }
    } else if (name.indexOf("Word") == 0) {
      textareas[name] = current;
    } else if (name.indexOf("comment") > 0) {
      comments[name] = current;
    }
  }    

  for ( i in rdgroup_array ) {
    var foo = rdgroup_array[i];
    var objrdgroup = form[foo];
    var objrdgroupval = getCheckedValue(objrdgroup);
    var objrdcomment = comments[foo + 'comment'];
    var objrdword = textareas['Word' + foo];
    if(objrdgroupval==''){
      objrdcomment.parentNode.style.display = "none";
    }

    else if(objrdgroupval== 1){
      objrdcomment.parentNode.style.display = "block";
      objrdword.value = "Action Plan:";
    }

    else if(objrdgroupval== 2){
      objrdcomment.parentNode.style.display = "block";
      objrdword.value = "Action Plan:";
    }


    else if(objrdgroupval== 4){
      objrdcomment.parentNode.style.display = "block";
      objrdword.value = "Comment:";
    }

    else if(objrdgroupval== 5){
      objrdcomment.parentNode.style.display = "block";
      objrdword.value = "Comment:";
    }
    else {objrdcomment.parentNode.style.display = "none";
    }
  }
}
0
 
Michel PlungjanIT ExpertCommented:
How about this:
nums = new Array()
nums["B"] = .50;
nums["C"] = .35;
nums["D"] = .15;
.
.


document.evaluation.elements[prefix+'average'].value = avg;
var weighted =  Math.round ( (sum[prefix] / count[prefix] * nums[prefix])*100)/100;
if(isNaN(weighted)) {
  document.evaluation.elements[prefix+'weighted'].value = 0;
  return false;
}
document.evaluation.elements[prefix+'weighted'].value = weighted;

instead of the if (prefix = ....) lines


and You should look at the switch statement for your display code:


dispStyle = "block";
switch  (objrdgroupval) {
    case 1 :
    case 2:
      objrdword.value = "Action Plan:";
      break
    case 4:
    case 5:
      objrdword.value = "Comment:";
      break
    default: dispStyle = "none";
}
objrdcomment.parentNode.style.display = dispStyle;

Michel
0
 
anthonywjones66Commented:
I don't have anything to add to the various improvements already posted but I thought for clarity I might state what really makes the difference.

Javascript under IE is not slow accessing the IE DOM is.

Take a look at this line from the original question:-

for (xx=0; xx < document.evaluation.elements.length; xx++)

When the condition is tested javascript has to dip into the DOM 3 time each access into the DOM is expensive relative other similar actions in javascript.

now take a look at this from one of  mplungjan's comments

myRadios=null;
function countRadios() {
  myRadios new Array()
  var cnt=0; // can use myRadios.length each time but that is also costly
  var elems=document.evaluation.elements;
  for (var i=0, n=elems.length;i < n; i++) {
    if (elems[i].type == 'radio') {
      myRadios[cnt++] = elems[i];
    }
  }
}

This is much better the condition now only dips into the DOM once per test. I'm not sure that I agree that accessing the length property of a javascript array is expensive being a purely javascript operation i doubt it costs anything at all.  Far more expensive is the elems.length.  This may be bettter:-

var myRadios
function countRadios() {
  myRadios = new Array()
  var elems=document.evaluation.elements;
  var cElems = elems.length
  for (var i=0, i < cElems ; i++) {
    if (elems[i].type == 'radio') {
      myRadios.push(elems[i])
    }
  }
}

This hits the DOM even less.

Basically if you have potentially large loop and you access a propery of the DOM more than once pull it into it's own variable (as long as it isn't one that changes during the loop).  This will make a massive difference to 'Javascript' perfromance.


BTW,  'for (var i=0, n=elems.length;i < n; i++) {'  what assing elems.length to n when the only use of n is to compare it with i imeadiately after???? and where is the var for n?

Anthony.

 
0
 
Michel PlungjanIT ExpertCommented:
this
 for (var i=0, n=elems.length;i < n; i++) {
is EXACTLY the same as this
 var cElems = elems.length
  for (var i=0; i < cElems ; i++) {

the for statement is
for (pre processing ; while something ; after each loop)
or more formal:
for (initialization; condition; increment)

Michel
PS: You forgot a semicolon after the i=0
PPS: 1:40am here. Time to sleep
0
 
anthonywjones66Commented:
mplungjan,

My apologies (it's 00:40am here) eyes seeing var i=0; n=elems.length, i< n :)

Anthony.
0
 
davidlars99Commented:
there's a speed test algorithm made in javascript which tells you to what number of milliseconds you'll need to set function time interval, for example you have created a game on PC that had 3.5 GHz and there was some action which required 500 as a time interval and when you tested it on a PC with 1.5 GHz or slower script ran terrible slow, this algorithm takes speed number argument as a rate per frame and calculates exact time interval to use on any machine for the original speed to run

http://www.frozendev.com/temp/speedtest.htm
0
 
davidlars99Commented:
and browsers other than IE seem slow... as Anthon said something about IE accessing DOM, not true... I don't care what IE does it's always runs javascript faster then any other browser
0
 
anthonywjones66Commented:
David,

>>
as Anthon said something about IE accessing DOM, not true... I don't care what IE does it's always runs javascript faster then any other browser
<<

I wasn't comparing IE with other browsers I was comparing operations in javascript with operations that access the IE DOM.  I've never compared accessing other browsers DOM's with how fast IE can access it's DOM so I can't comment there.  You may well be right that IE is faster however accessing _any_ DOM is still way slower than pure JScript operations.

Anthony.
0
 
searlasCommented:
mplungjan,

---
this
 for (var i=0, n=elems.length;i < n; i++) {
is EXACTLY the same as this
 var cElems = elems.length
  for (var i=0; i < cElems ; i++) {
---

Come on... you know better than that.  elems is a collection on the form, not an Array.  Accessing the length property causes Internet Explorer to check the number of elements in the form (in case any have been added dynamically.)  i.e. every time you access the length property it actually goes and counts the number of elemets in the form.  The second version (anthonywjones66's) is MUCH faster.

http://msdn.microsoft.com/library/default.asp?url=/workshop/author/dhtml/reference/collections/elements.asp
http://msdn.microsoft.com/workshop/author/perf/perftips.asp#Optimize_Your_Scripts


0
 
anthonywjones66Commented:
searlas,

Look real carefully at where the commas and semi-colons are.  I missed it originally but mplungjan is absolutely right.

Anthony.
0
 
Michel PlungjanIT ExpertCommented:
Yeah... A comma in JS can be important ;)
0
 
searlasCommented:
Damned comma operator...... yeah, my bad.

I wouldn't have embarassed myself if y'all just stopped posting after I fixed the speed issue...
;-)
0
 
virmaiorCommented:
searles: your solution isn't fundamentally different than mine.  I don't really see how you fixed the speed issue
mplungjan: yeah my code was sloppy and taking the .length out of the loop should speed it up (because just becaue it is a collection doesn't mean that MS IE actually followed that rule for having the length alredy be figured out).

further using slurpy/countRadios (my code and then mplungjan's cleaner version of the same) would be faster than any of the other solutions because the other solutions still scroll across all elements every time rather than just at page load.
0
 
searlasCommented:
Hi virmajor,

My comment 'I fixed the speed issue' just meant I indepently wrote some code and posted some cut'n'paste functions that could be used in place of the ones currently used at
http://evalplus.webasics.net/javascriptissue.html

Sure, slurpy/countRadios is a good way to go, and if you'd finished the job I probably wouldn't have bothered with my code at all.  But with comments like 'not tested' and 'there may be syntax errors' it kind of implied that you hadn't actually checked whether your code (a) worked, and (b) was any faster.

'my guess is there are far fewer radios than there are total elements'... did you look at the page?  radio buttons account for more than half the elements.  So yes, there are 'far fewer', but your reasoning is faulty.  The reason iterating over a separate array of radio buttons is quicker is because you're reducing/eliminating the dom access... not because the number of loops has been reduced.  And when you realise that it's the DOM access that's the performance killer, you have to admit your first posts including repeated use of document.evaluation.elements are actually pretty poor.

You're probably right that yours+mplungjan's ideas would lead to the fastest solution.  The problem we were working on was to reduce a page initialisation time > 60 seconds, and an onclick time of ~ 17 seconds.  I actually checked my code and both times were reduced to acceptable levels.  You didn't even take the effort to syntax check yours.


0
 
virmaiorCommented:
true enough.
0
 
LManesseAuthor Commented:
This post certainly got a bit lively.  Thanks to all who posted.  I don't think I've had this many responses to a post before, so this has been a very worthwhile and learning experience for me.  

I think I need to agree with searlas.  Being the javascript novice that I am, I tried to implement both virmajor and mplungjan's  ideas, but couldn't get them to work on my page with the exception of one of virmajor's suggestion that knocked about 20 seconds off page load.

I was able to easily implement searlas' code and it radically reduced the page load time.  The whole page now loads in 3 seconds instead of the original 118 seconds.  You can check it out if you wish:  http://evalplus.webasics.net/javascriptissue.html .

Thanks searlas.


0

Featured Post

Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

  • 9
  • 7
  • 5
  • +3
Tackle projects and never again get stuck behind a technical roadblock.
Join Now