Ajax problem
in [JavaScript]
|
Prev: Tramadol online without doctor prescription, cheap online pharmacy Tramadol, Buy Tramadol order
Next: ![] == false returns true
From: Jorge on 26 Mar 2010 11:44 On Mar 26, 2:17 pm, Hans-Georg Michna <hans- georgNoEmailPle...(a)michna.com> wrote: > On Wed, 24 Mar 2010 17:02:54 -0700 (PDT), Mike wrote: > >Great, I Took the alerts out and it works. > >Why does that happen? > > Perhaps because the readystatechange event had already fired for > the last time before you clicked on the alert's OK button. Yep. > Checkhttp://winhlp.com/node/684for a functioning ajax routine > that has all the needed little stuff already. (...) No, please, no. That's a good example of how to turn 3-4 LOC into 106... (Why ?) -- Jorge.
From: Hans-Georg Michna on 27 Mar 2010 08:18 On Fri, 26 Mar 2010 08:44:24 -0700 (PDT), Jorge wrote: >No, please, no. That's a good example of how to turn 3-4 LOC into >106... (Why ?) What does that mean? I'm very interested when I hear about possible improvements. Hans-Georg
From: kangax on 27 Mar 2010 12:43 On 3/27/10 8:18 AM, Hans-Georg Michna wrote: > On Fri, 26 Mar 2010 08:44:24 -0700 (PDT), Jorge wrote: > >> No, please, no. That's a good example of how to turn 3-4 LOC into >> 106... (Why ?) > > What does that mean? > > I'm very interested when I hear about possible improvements. Glancing quickly, I see that `activexmodes` array is created every time `ajax` is called. This is not necessary. It kills runtime performance and hogs garbage collector with additional work. You're calling `setRequestHeader`, but don't check if it exists (I don't know which implementations don't have it, but it shouldn't matter). The case with IE returning 1223 status code (check the archives for more info) doesn't seem to be handled. Also to me, method named `ajax` looks a bit vague. And having a function with 8(!) arguments usually means that it's time to "introduce parameter object" (<http://www.refactoring.com/catalog/introduceParameterObject.html>), or perform some similar refactoring. There are probably other things, which I overlooked. -- kangax
From: Hans-Georg Michna on 28 Mar 2010 14:56
Thanks a lot! On Sat, 27 Mar 2010 12:43:59 -0400, kangax wrote: >Glancing quickly, I see that `activexmodes` array is created every time >`ajax` is called. This is not necessary. It kills runtime performance >and hogs garbage collector with additional work. It is not quite so bad. The array is created in the XHR() constructor, but the ajax.xhr object created from that constructor is only created once and then re-used. Thus the array is actually only created once. >You're calling `setRequestHeader`, but don't check if it exists (I don't >know which implementations don't have it, but it shouldn't matter). > >The case with IE returning 1223 status code (check the archives for more >info) doesn't seem to be handled. > >Also to me, method named `ajax` looks a bit vague. Will work on these points. I wasn't even aware of the existence of status 1223. In its current form the ajax routine doesn't do much with the status codes. The user-supplied processAjaxResult routine has to handle them. >And having a function with 8(!) arguments usually means that it's time >to "introduce parameter object" >(<http://www.refactoring.com/catalog/introduceParameterObject.html>), or >perform some similar refactoring. The last couple of arguments are rarely used, so it isn't quite so bad. I don't find the figure of 8 too fearful either. If it had 20 parameters, I would tend to agree. (:-) >There are probably other things, which I overlooked. If you happen to notice anything else, please let me know. Hans-Georg |