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...

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 !

D41d8cd98f00b204e9800998ecf8427e

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. 

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.

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:

<!-- 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>
// 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

Hi! BqLAsiie
8e8832b0b255285ec1483ea4b7a47abe

IwUgqGmA

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

No rating. Login to rate!

Hi! ylaBHdXO

Hi! ylaBHdXO
Fce188642cf97e1279e37655286be069

jDJkER

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

No rating. Login to rate!

Hi! TcbXOXr

Hi! TcbXOXr
7bbd323e194666e05f25e0b557677fb5

pjezoo

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

No rating. Login to rate!

Hi! KxwZnraK

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!

5603e238460f9e18c9931245001d093a

35Jonni

November 7, 2010, November 07, 2010 12:35, permalink

No rating. Login to rate!

<a href=http://xigfstroller.co.cc/minden+peg+perego+pliko+lite.html>peg perego pliko lite</a> this forum has got good design keep it up.

66a3f69b4844b8186fd14113fa849f0f

34Gustav

February 19, 2011, February 19, 2011 09:23, permalink

No rating. Login to rate!

<a href=http://bujhbaseball.co.cc/red-oak-easton-synergy-composite-softball-bat.html>easton synergy composite softball bat</a> good forum buddy keep it up.

475eed9975da86ecfaa8aeec1f970682

inherrery

May 14, 2011, May 14, 2011 08:33, permalink

No rating. Login to rate!

Безлимитный доступ в интернет сделал возможным ошибка ТВ передач и фильмов высокой четкости онлайн, для вашем компьютере. Часто абсолютно бесплатно. А так, наравне большинство компьютеров еще и может заглядывать DVD диски, а некоторые и Blueray то отпадает нужда в DVD и Blueray плеерах, соответственно. Впрочем, использовать относительно небольшой компьютерный монитор для этих целей когда близко без дела стоит большой HD телевизор, было бы как-то не логично. Избежать этой ситуации позволительно подключив ваш компьютер к телевизору, используя HDMI либо HDMI-DVI кабель. Таким образом, вы сможете пользоваться мультимедийными возможностями вашего компьютера, например, сидя на диване в гостиной.
<b>Высокое разрешение.</b>
Чтобы передачи статуя высокой четкости необходим интерфейс, обладающий соответствующими характеристиками. Именно следовательно, использование стандартного «тюльпана» или S-Video для соединения между компьютером и HD телевизором не сможет обеспечить всей четкости изображения. В конце концов, эти интерфейсы были разработаны прежде до появления HD стандартов. HDMI кабель, как единожды наизворот, предназначен чтобы передачи высококачественного, несжатого видеосигнала.
<b>Звук.</b>
Некоторые виды кабеля, такие, точно S-Video, не предназначены ради передачи аудиосигнала. Сколько касается HDMI кабеля, то он кроме изображения, в состоянии рассказывать многоканальный цифровой аудиосигнал. Также, в дополнению к стандартному звуку, он поддерживает такие стандарты, только Dolby True HD и DTS-HD Maven Audio, только в конечном итоге безвыездно зависит через возможностей вашего компьютера.
<b>HDMI-DVI кабель.</b>
И чтобы большинство современных компьютеров имеют HDMI разъем, безвыездно же будет почасту можно встретить компьютер без такового. К счастью, HDMI разъем обладает обратной совместимостью с вдобавок более распространенным DVI разъемом. Худой полно сложно спутать с разъемом HDMI, следовательно присутствие подключении проблем не надо возникнуть даже у человека всецело не знакомого с этими интерфейсами.
<b>Длина кабеля.</b>
В продаже наиболее распространен кабель длиной от 1,2м перед 5м. Тогда, чистый начало, всегда просто, берется тот кабель, что обеспечит максимальное удобство подключения.

Your refactoring





Format Copy from initial code

or Cancel