A969cbe277419ae0af0acee58f4cfa15

This submission is from a question I received on Javascript Kata (http://www.javascriptkata.com/2007/12/06/ask-dan-more-on-javascript-threading/). Please read the post first and then try to refactor the code...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
function startup() {
  setInterval(”pinger(’live’,0)”,10000);
  setInterval(”pinger(’standby’,1)”,10000);
  setInterval(”pinger(’dev’,2)”,10000);
  setInterval(”pinger(’test1′,3)”,10000);
  setInterval(”pinger(’test2′,4)”,10000);
  setInterval(”pinger(’test3′,5)”,10000);
  setInterval(”pinger(’test4′,6)”,10000);
}

function pinger(server,divit) {
  console=document.getElementById(’ping_div’ + divit);
  console.innerHTML=”;
  sendRequest(”../php/ping_sys2.php?target=+ server);
}

Refactorings

No refactoring yet !

Avatar

Casper

December 6, 2007, December 06, 2007 16:45, permalink

No rating. Login to rate!

Are you sure you don't have a race condition sneaking in there somewhere in sendRequest()? I'd like to see the implementation of that method to really make sure. No global variables or something obvious like that?

Bbcf8931a1494ef9ac6b9ca774ef41df

BK

December 6, 2007, December 06, 2007 22:42, permalink

No rating. Login to rate!

I agree with Casper that this looks a lot like the sendRequest method would always use the same XMLHTTP object and so, by getting the alert message, the request has enough time to come back before a new one takes ownership of the onreadystate handler.

425569e03b2ca45336670d8905f2ae6e

Stuart

December 7, 2007, December 07, 2007 11:09, permalink

No rating. Login to rate!

The sendRequest object. Since reading about the global var I have tried removing the "var xmlReq;" line from code and there is no difference in behaviour. The final call still gets executed while the rest stay on "sending". I can certainly see the sense in the suggestion that the onreadystate handler isn't having enough time to complete its execution.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
var xmlReq;

function sendRequest(url, params, HttpMethod){

if(!HttpMethod) {
HttpMethod=”POST”;
}

xmlReq=startXMLHTTPRequest();

if(xmlReq) {

xmlReq.onreadystatechange=onReadyState;
xmlReq.open(HttpMethod,url,true);
xmlReq.send(params);
}
}

function startXMLHTTPRequest(){

var req=null;

try {
req=new ActiveXObject(”Microsoft.XMLHTTP”);
}
catch(e)
{
req=new XMLHttpRequest()
}

return req;

}

function onReadyState()
{
var readyXML=xmlReq.readyState;
var data=null;

if(readyXML == 4){
if(xmlReq.status == 200) {
console.innerHTML=”;
data=xmlReq.responseText;
}
else {
data=xmlReq.status;
}
}
else{
console.innerHTML=”;

if(readyXML == 3){
data=”Serving …”;
}
if(readyXML == 2){
data=”Loading …”;
}
if(readyXML == 1){
data=”Sending …”;
}
}

dataInnerHTML(data);
}

function dataInnerHTML(data){

if(data!=null){
var newline=document.createElement(”div”);
console.appendChild(newline);
newline.innerHTML+="+data+”";
}

}
Bbcf8931a1494ef9ac6b9ca774ef41df

BK

December 7, 2007, December 07, 2007 15:59, permalink

No rating. Login to rate!

First thing, even though you don't define the global var, since you probably didn't add var in front of xmlReq=startXMLHTTPRequest();
it is still considered a global variable. This means that when you test readyState, you don't necessarily test the one that is really doing the callback but now that I think about it, I don't think the problem is only there. Console is also defined as global! This means that everything is written to the las div called. Maybe you should use closures.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
function startup() {
    setInterval("pinger('live',0)",10000);
    setInterval("pinger('standby',1)",10000);
    setInterval("pinger('dev',2)",10000);
    /*setInterval("pinger('test1',3)",10000);
    setInterval("pinger('test2',4)",10000);
    setInterval("pinger('test3',5)",10000);
    setInterval("pinger('test4',6)",10000);
    */
}

function pinger(server,divit) {
    var data = new Object();
    data.console=document.getElementById('ping_div' + divit);
    data.console.innerHTML="";
    sendRequest("http://www.google.com", undefined, undefined, data);
}


function sendRequest(url, params, HttpMethod, data){
    if(!HttpMethod) {
        HttpMethod="POST";
    }

    var xmlReq=startXMLHTTPRequest();

    if(xmlReq) {
        xmlReq.onreadystatechange=function(){onReadyState(xmlReq, data)}
        xmlReq.open(HttpMethod,url,true);
        xmlReq.send(params);
    }
}

function startXMLHTTPRequest(){
    var req=null;
    
    try {
        req=new ActiveXObject("Microsoft.XMLHTTP");
    }catch(e){
        req=new XMLHttpRequest()
    }
    
    return req;
}

function onReadyState(xmlReq, data){
    
    switch (xmlReq.readyState){
        case 4:
            if(xmlReq.status == 200) {
                console.innerHTML="";
                data.txt=xmlReq.responseText;
            }else {
                data.txt=xmlReq.status;
            }
            break;
        case 3:
            data.txt="Serving …";
            break;
        case 2:
            data.txt="Loading …";
        case 1:
            data.txt="Sending …";
    }
    
    dataInnerHTML(data);
}

function dataInnerHTML(data){
    if(data!=null && data.txt!=null){
        var newline=document.createElement("div");
        data.console.appendChild(newline);
        newline.innerHTML+=""+data.txt+"";
    }
}
B849433e0e1a3f4ca0cf7cc55b8acd53

Casper

December 7, 2007, December 07, 2007 16:54, permalink

No rating. Login to rate!

Like BK said you have global variables all over the place and this breaks the code. You have to think about what happens when your methods are called concurrently. For example "console" will be overwritten by the latest call, since it's a global variable.

But, the number 1 rule of coding is don't reinvent the wheel. XMLHTTPRequests have been done a million times before, and there are very good libraries that wrap this stuff into much easier packages. For example the solution by using the excellent jQuery library:

Html snippet

1
2
3
4
5
6
<!-- Import jQuery -->
<script src="http://jquery.com/src/jquery-latest.js" type="text/javascript"></script>  

<div id="ping_div_live"></div>
<div id="ping_div_standby"></div>

Javascript part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
// Untested code, but hopefully you get the idea
function startup() {
  ping('live',    10000);
  ping('standby', 10000);
  // ...
}

function ping(hostname, interval) {
  setInterval(function() { do_ajax_ping(hostname); }, interval);
}

function do_ajax_ping(hostname) {
  // Using the jQuery AJAX method here, makes this easy
  $.ajax({
      type: "GET",
      url: "../php/ping_sys2.php?target=" + hostname,
      processData: false,

      // Using a closure to be called on success here
      success: function(data) {  
        // jQuery DOM manipulation. Again..easier like this.
        $("#ping_div_" + hostname).append("<div>" + data + "</div>");
      }
   });
}
425569e03b2ca45336670d8905f2ae6e

Stuart

December 10, 2007, December 10, 2007 14:18, permalink

No rating. Login to rate!

Thanks for the responses ... as I said I wrote my own handler as an exercise in learning. Probably once having done it to understand the process, I should have have switched back to a standard library. I would never have recognised (without your help) the added oo element, which is now my focus for understanding completely. I hate being in a situation where I am dependant on other code without knowing how it works (I'm strange/pedantic like that).

Thanks for your help and I will wander off and delve the object intricacies that have been thrown up.

7e2bdbfc41ced1929ffa3fcd8f7d2f6c

YpWMlCK

November 18, 2009, November 18, 2009 22:08, permalink

No rating. Login to rate!

Hi! BqLAsiie

1
Hi! BqLAsiie
8e8832b0b255285ec1483ea4b7a47abe

IwUgqGmA

November 18, 2009, November 18, 2009 23:02, permalink

No rating. Login to rate!

Hi! ylaBHdXO

1
Hi! ylaBHdXO
Fce188642cf97e1279e37655286be069

jDJkER

November 19, 2009, November 19, 2009 19:46, permalink

No rating. Login to rate!

Hi! TcbXOXr

1
Hi! TcbXOXr
7bbd323e194666e05f25e0b557677fb5

pjezoo

November 19, 2009, November 19, 2009 20:51, permalink

No rating. Login to rate!

Hi! KxwZnraK

1
Hi! KxwZnraK
6b84900961a9e82da5d7ae29ff8fab6c

diabkoxp18

January 14, 2010, January 14, 2010 04:17, permalink

No rating. Login to rate!

So I just got a phone call from my girl friend that she has been fucking around with my friend!!! So like any angry boy friend I have choosen to download every hot video I have of her to the internet. I have posted the pics here: http://www.GF4Free.com/
Just search for sexyerica344, enjoy!

6b84900961a9e82da5d7ae29ff8fab6c

diabkoxp92

January 29, 2010, January 29, 2010 13:12, permalink

No rating. Login to rate!

So I just got a call from my girl friend that she has been sleeping around with my brother!!! So like any angry boy friend I have decided to share every nude picture I have of her to the net. I have posted the pics here: http://www.GF4Free.com/
Just search for sexyerica344, enjoy!

Your refactoring





Format Copy from initial code

or Cancel