AJAX methodology

[getsanjay.sharma]

Hello to all Javascript programmers out there.

I have written a small Ajax module which makes use of Object literals
to do it's work. It is split into two files, an Ajax.js which contains
all the ajax related functionality and a 'web page' specific .js file
which contains the logic required for the page and which makes use of
'Ajax.js'. Though looking at my implementation I feel as though there
is a lot of coupling. I would like to get your views regarding this
approach used by me.

Any suggestions on what changes I would make to this framework to make
it any better?

AJAX.js

var AJAX =
{
xhr: null,
method: "GET",
url: "",
createXHRObject : function()
{
if(window.ActiveXObject)
return(new ActiveXObject("Microsoft.XMLHTTP"));
else
return(new XMLHttpRequest());
},

setParameters: function(url, method)
{
if(typeof url != 'undefined' && url != null) this.url = url;// + "?
timeStamp=" + new Date().getTime();
if(typeof method != 'undefined' && method != null) this.method =
method;
},

getResponse : function(url, method, handler)
{
alert('h');
this.setParameters(url, method);
this.xhr = this.createXHRObject();
handler['xhr'] = this.xhr;
this.xhr.onreadystatechange = handler;
this.xhr.open(this.method, this.url, true);
this.xhr.send(null);
}
};

SomeForm.js

var SS =
{
setAttributes: function(elem, type, statusId)
{
this.handler['elem'] = elem;
this.handler['type'] = type;
this.handler['statusElem'] = document.getElementById(statusId);
},

/** The reason we can't use 'this' inside handler since it would not
refer to the properties of SS but to the function handler since it is
used as a callback function and inside callbacks, this refers to the
function itself.
*/
handler : function()
{
var XHR = XHR || this['xhr'];
var elem = elem || this['elem'];
var type = type || this['type'];
var statusElem = statusElem || this['statusElem'];
if(XHR.readyState == 4)
{
if(XHR.status == 200)
{
var result = XHR.responseText;
var target = elem.nextSibling;
statusElem.innerHTML = 'Done';
if(result === 'true')
target.innerHTML = "Successful";
else
target.innerHTML = result;
}
else
{
statusElem.innerHTML = 'Invalid url specified / Resource not
found';
}
}
},

/* It is necessary to encode all the input which comes from teh user
since any invalid character will cause the query string to go bad and
the fetching ofdata from the server fail. In short if you are
constructing your querystring based on teh data fetched by the user,
encodeURIComponent is a must!!! */

validate : function(elem, type, statusId)
{
this.setAttributes(elem, type, statusId);
var url = "?do=validate&type=" + encodeURIComponent(type)
+ "&value=" + encodeURIComponent(elem.value);
AJAX.getResponse(url, null, this.handler);
}
};

Any pointers, suggestions to make this better would really be
appreciated.

Thanks and regards,
S T S

 

[Thomas 'PointedEars' Lahn]

Any suggestions on what changes I would make to this framework to make
it any better?

AJAX.js

var AJAX =

There is no good reason not to wrap this into a prototype object.

{
xhr: null,
method: "GET",
url: "",
createXHRObject : function()
{
if(window.ActiveXObject)

Insufficient. You have to make sure that ActiveXObject can be [[Construct]]ed.

return(new ActiveXObject("Microsoft.XMLHTTP"));

Insufficient. You have to handle the case where this throws an exception.

else
return(new XMLHttpRequest());

Insufficient. Lack of support for window.ActiveXObject does not imply
support for XMLHttpRequest. You have to make sure that XMLHttpRequest can
be [[Construct]]ed.

},

setParameters: function(url, method)
{
if(typeof url != 'undefined' &&

Unnecessary. `url' is declared per the argument list, and

url != null)

will also yield `false' if `url' is `undefined'. Therefore:

if (url && typeof url == "string")

this.url = url;// + "?
timeStamp=" + new Date().getTime();

This is a syntax error.

if(typeof method != 'undefined' && method != null) this.method =
method;

Same here.

},

getResponse : function(url, method, handler)
{
alert('h');

That line should only be left from debugging.

this.setParameters(url, method);
this.xhr = this.createXHRObject();
handler['xhr'] = this.xhr;

No need for the bracket the property accessor syntax here.

handler.xhr = this.xhr;

this.xhr.onreadystatechange = handler;
this.xhr.open(this.method, this.url, true);
this.xhr.send(null);

When `this.method' is "POST", you have to send something here different from
`null'.

}
};

SomeForm.js

var SS =
{
setAttributes: function(elem, type, statusId)
{
this.handler['elem'] = elem;
this.handler['type'] = type;
this.handler['statusElem'] = document.getElementById(statusId);

You have to make sure that document.getElementById() can be called before
you call it.

},

/** The reason we can't use 'this' inside handler since it would not
refer to the properties of SS but to the function handler since it is
used as a callback function and inside callbacks, this refers to the
function itself.
*/

The observation is correct, the reasoning is flawed.

handler : function()
{
var XHR = XHR || this['xhr'];

There is no point in this Boolean expression. `XHR' will always have the
value `undefined' (after local variable instantiation), so the above is
semantically equal to

var XHR = this['xhr'];

var elem = elem || this['elem'];
var type = type || this['type'];
var statusElem = statusElem || this['statusElem'];

Same here.

if(XHR.readyState == 4)
{
if(XHR.status == 200)
{
var result = XHR.responseText;
var target = elem.nextSibling;
statusElem.innerHTML = 'Done';

The proprietary `innerHTML' property has been proven also to be error-prone
and should not be used. Certainly it should not be hard-coded into a library.

if(result === 'true')

That is far too specialized for the library, depending on the value of a
specific response, effectively ruling out all other possible responses.

[...]
Any pointers, suggestions to make this better would really be
appreciated.

HTH


PointedEars

 

[getsanjay.sharma]

There is no good reason not to wrap this into a prototype object.

You mean create a class? I didn't think about a class considering I
would be using only one AJAX event at a time though when you say it,
it seems a bit restrictive. So class is it?

{
xhr: null,
method: "GET",
url: "",
createXHRObject : function()
{
if(window.ActiveXObject)

Insufficient. You have to make sure that ActiveXObject can be [[Construct]]ed.

return(new ActiveXObject("Microsoft.XMLHTTP"));

Insufficient. You have to handle the case where this throws an exception.

else
return(new XMLHttpRequest());

Insufficient. Lack of support for window.ActiveXObject does not imply
support for XMLHttpRequest. You have to make sure that XMLHttpRequest can
be [[Construct]]ed.

},

setParameters: function(url, method)
{
if(typeof url != 'undefined' &&

Unnecessary. `url' is declared per the argument list, and

url != null)

will also yield `false' if `url' is `undefined'. Therefore:

if (url && typeof url == "string")

this.url = url;// + "?
timeStamp=" + new Date().getTime();

This is a syntax error.

if(typeof method != 'undefined' && method != null) this.method =
method;

Same here.

},

getResponse : function(url, method, handler)
{
alert('h');

That line should only be left from debugging.

this.setParameters(url, method);
this.xhr = this.createXHRObject();
handler['xhr'] = this.xhr;

No need for the bracket the property accessor syntax here.

handler.xhr = this.xhr;

this.xhr.onreadystatechange = handler;
this.xhr.open(this.method, this.url, true);
this.xhr.send(null);

When `this.method' is "POST", you have to send something here different from
`null'.

}
};

SomeForm.js

var SS =
{
setAttributes: function(elem, type, statusId)
{
this.handler['elem'] = elem;
this.handler['type'] = type;
this.handler['statusElem'] = document.getElementById(statusId);

You have to make sure that document.getElementById() can be called before
you call it.

},

/** The reason we can't use 'this' inside handler since it would not
refer to the properties of SS but to the function handler since it is
used as a callback function and inside callbacks, this refers to the
function itself.
*/

The observation is correct, the reasoning is flawed.

So what would the correct reasoning be? So inside a callback, doesn't
this refer to the current function?

handler : function()
{
var XHR = XHR || this['xhr'];

There is no point in this Boolean expression. `XHR' will always have the
value `undefined' (after local variable instantiation), so the above is
semantically equal to

var XHR = this['xhr'];

var elem = elem || this['elem'];
var type = type || this['type'];
var statusElem = statusElem || this['statusElem'];

Same here.

if(XHR.readyState == 4)
{
if(XHR.status == 200)
{
var result = XHR.responseText;
var target = elem.nextSibling;
statusElem.innerHTML = 'Done';

The proprietary `innerHTML' property has been proven also to be error-prone
and should not be used. Certainly it should not be hard-coded into a library.

if(result === 'true')

That is far too specialized for the library, depending on the value of a
specific response, effectively ruling out all other possible responses.

I used innerHTML and 'true' literal since the javascript code other
than Ajax.js is application specific and there has to be some kind of
hardcoding somewhere.

Here are my updated javascript files.

var AJAX =
{
xhr: null,
method: "GET",
url: "",
createXHRObject : function()
{
try
{
if(window.ActiveXObject)
return(new ActiveXObject("Microsoft.XMLHTTP"));
else if(XMLHttpRequest)
return(new XMLHttpRequest());
else
{
alert("Your browser doesn't support AJAX");
return(null);
}
}
catch(e)
{
alert("An exception has occured!");
return(null);
}
},

setParameters: function(url, method)
{
this.url = url;
this.method = method;
},

getResponse : function(url, method, handler, data)
{
this.setParameters(url, method);
this.xhr = this.createXHRObject();
handler.xhr = this.xhr;
this.xhr.onreadystatechange = handler;
this.xhr.open(this.method, this.url, true);
if(this.method == "POST)
this.xhr.send(data);
else
this.xhr.send(null);
}
};

=========================================

var SS =
{
setAttributes: function(elem, type, statusId)
{
//
this.handler['elem'] = elem;
this.handler['type'] = type;
if(document.getElementById(statusId))
this.handler['statusElem'] = document.getElementById(statusId);
},

handler : function()
{
var XHR = this['xhr'];
var elem = this['elem'];
var type = this['type'];
var statusElem = this['statusElem'];
if(XHR.readyState == 4)
{
if(XHR.status == 200)
{
var result = XHR.responseText;
var target = elem.nextSibling;
statusElem.innerHTML = 'Done';
if(result === 'true')
target.innerHTML = "Successful";
else
target.innerHTML = result;
}
else
{
statusElem.innerHTML = 'Invalid url specified / Resource not
found';
}
}
else
{
statusElem.innerHTML = 'Loading. Please wait...';
}
},

validate : function(elem, type, statusId)
{
this.setAttributes(elem, type, statusId);
var url = "?do=validate&type=" + encodeURIComponent(type)
+ "&value=" + encodeURIComponent(elem.value);
AJAX.getResponse(url, null, this.handler, null);
}
};

Any comments on the design and design changes to the same would be
really appreciated.
Thanks and regards,
STS

 

[Thomas 'PointedEars' Lahn]

You mean create a class? I didn't think about a class considering I
would be using only one AJAX event at a time though when you say it,
it seems a bit restrictive. So class is it?

Sigh. [psf 10.1]

Of course not. I meant what I wrote. The language(s) you are using
(client-side) has/have no concept of classes, it/they use(s) prototype-based
inheritance. The underlying concept is the first thing to know about a
programming language.

http://javascript.crockford.com/javascript.html

[...]

Please trim your quotes as recommended by the http://jibbering.com/faq/

I used innerHTML and 'true' literal since the javascript code other
than Ajax.js is application specific and there has to be some kind of
hardcoding somewhere.

But not there. At that point you should call a user-defined callback method
instead.

Here are my updated javascript files.

var AJAX =
{
xhr: null,
method: "GET",
url: "",
createXHRObject : function()
{
try
{
if(window.ActiveXObject)
return(new ActiveXObject("Microsoft.XMLHTTP"));

Nothing has changed here. You are still not testing properly if generally
ActiveXObject can be [[Construct]]ed, and you don't handle the exception if
the construction throws an exception.

else if(XMLHttpRequest)
return(new XMLHttpRequest());

Here it only went worse. If `XMLHttpRequest' is not defined, the code will
simply break. It should be something along

if (isMethodType(typeof XMLHttpRequest) && XMLHttpRequest)
{
try
{
return new XMLHttpRequest();
}
catch (e)
{
// ...
}
}

As XMLHttpRequest is currently supported more and may continue to be due to
its being in the process of being standardized by the W3C[1], it should take
precedence over the use of ActiveXObject.

[1] http://www.w3.org/TR/XMLHttpRequest/

else
{
alert("Your browser doesn't support AJAX");
return(null);

`return' is _not_ a method. Unless absolutely required (such as for
multi-line expressions), do not use parentheses for its (optional)
parameter. And if you use parens, delimit the parameter from the keyword by
whitespace, so that it is clearly marked as not being a method call. (Much
the same goes for other control statements like `if', `for', `while' aso.)

return null;

or, if you must,

return (null);

The statement in the alert() is wrong, as other browsers support "AJAX"
differently (IceBrowser uses window.createRequest()). And this should be
at least a (silent) exception, not an alert(). Such an error message is
of exactly no use to the user.

}
}
catch(e)
{
alert("An exception has occured!");
return(null);

Same here.

[...]
var SS =
{
setAttributes: function(elem, type, statusId)
{
//
this.handler['elem'] = elem;
this.handler['type'] = type;
if(document.getElementById(statusId))
this.handler['statusElem'] = document.getElementById(statusId);

You made it worse. Now document.getElementById() is needlessly called
twice. I recommended you should test whether it can be called or not
instead. Such as in

if (document.getElementById)
{
this.handler['statusElem'] = document.getElementById(statusId);
}

or better:

if (isMethodType(typeof document.getElementById)
&& document.getElementById)
{
this.handler['statusElem'] = document.getElementById(statusId);
}

Since that test is always the same, it may suffice to do it only once,
assuming that the DOM will not be crippled by a script in the meantime.

[...]
Any comments on the design and design changes to the same would be
really appreciated.

You should consider *all* the advice that was given. If something is not
clear, you should ask about it.


PointedEars