patternjavascriptMinor
group - ajax - 0 - to_do - consistent access, styling
Viewed 0 times
ajaxto_dogroupconsistentstylingaccess
Problem
EDIT - 0 - Still need to fix markup language to make more unique so escaping is not needed:
This will be a JSON / JQuery Alternative.
```
/
group:ajax
/
/*
- good place to add mysql logging to track object usage and reshape code via machine learning
- ouptut - returns false or ajax_object
*/
function ajax_object()
{
var object;
try
{
object=new XMLHttpRequest();
}
catch(error_1)
{
alert('ajax_object : not instantiated:contact-support@archemarks.com : Error : ' + error_1)
}
return object;
}
/*
- 'path' holds the servers side function to respond to the request
- 'param' hold the serialized information to send the server
- 'ajax_func' holds the javascript function to respond to the server
- 'div' div holds the html location to send the response text
- Output: returns 1 for pass.
*/
function ajax(path,param,ajax_func,html_div)
{
var object=new ajax_object();
object.open("POST",path,true);
object.setRequestHeader("Content-type","application/x-www-form-urlencoded");
object.setRequestHeader("Content-length",param.length);
object.setRequestHeader("Connection","close");
object.onreadystatechange=function()
{
if(this.readyState===4)
{
if(this.status===200)
{
if(this.responseText!=null)
{
ajax_func(this.responseText,html_div);
}
else alert("ajax: responseText is null")
}
else alert('AJAX FAIL: Path - ' + path + ' .status = ' + this.status + ' . responseText = ' + this.responseText)
}
}
object.send(param);
return 1;
}
/*
*/
var aml_status_type =
{
"pass":0,
"fail":1,
"undefined":2
}
/*
*/
function check_aml(text)
{
var aml_response=patterns.aml.exec(text);
if(aml_response)
{
i
This will be a JSON / JQuery Alternative.
```
/
group:ajax
/
/*
- ajax_object() - creates a browser dependent ajax_object for the ajax method call
- good place to add mysql logging to track object usage and reshape code via machine learning
- ouptut - returns false or ajax_object
*/
function ajax_object()
{
var object;
try
{
object=new XMLHttpRequest();
}
catch(error_1)
{
alert('ajax_object : not instantiated:contact-support@archemarks.com : Error : ' + error_1)
}
return object;
}
/*
- ajax() - post style ajax
- 'path' holds the servers side function to respond to the request
- 'param' hold the serialized information to send the server
- 'ajax_func' holds the javascript function to respond to the server
- 'div' div holds the html location to send the response text
- Output: returns 1 for pass.
*/
function ajax(path,param,ajax_func,html_div)
{
var object=new ajax_object();
object.open("POST",path,true);
object.setRequestHeader("Content-type","application/x-www-form-urlencoded");
object.setRequestHeader("Content-length",param.length);
object.setRequestHeader("Connection","close");
object.onreadystatechange=function()
{
if(this.readyState===4)
{
if(this.status===200)
{
if(this.responseText!=null)
{
ajax_func(this.responseText,html_div);
}
else alert("ajax: responseText is null")
}
else alert('AJAX FAIL: Path - ' + path + ' .status = ' + this.status + ' . responseText = ' + this.responseText)
}
}
object.send(param);
return 1;
}
/*
- "enumeration" for aml_status_type
*/
var aml_status_type =
{
"pass":0,
"fail":1,
"undefined":2
}
/*
- check_aml()
*/
function check_aml(text)
{
var aml_response=patterns.aml.exec(text);
if(aml_response)
{
i
Solution
-
If you're just passing the IDs of elements, rather than the elements themselves, your variable names should reflect that. (Yes, it's possible to pass DOM elements around -- they're just objects, as far as JS cares. And if you passed the elements instead of their IDs, the code to do stuff with them would be shorter and you wouldn't feel the need for all those annoying
-
Observe:
IE6 includes MSXML v3, as does every OS since Win2k SP4. And v2, i believe, is supported as far back as IE5 and Win98. If your user is running something older than that, stuff should break. Gleefully. And
-
This "aml" stuff kinda creeps me out. If you intend for the response to be interpreted by JS, a more standard solution like JSON may be a better way to go. It's more portable (namely, you end up with built-in encoding/decoding in both JS and PHP), and you don't need to call a function like
-
In
The first two if sections should not exist, if you're not going to have anything in them. Only test for the conditions you actually intend to do something about. (If you actually had code there and took it out to lessen the amount of code you paste here, ok. Ignore the suggestion to remove. But for future reference, when you omit stuff, a comment like
-
-
I see references to
If you're just passing the IDs of elements, rather than the elements themselves, your variable names should reflect that. (Yes, it's possible to pass DOM elements around -- they're just objects, as far as JS cares. And if you passed the elements instead of their IDs, the code to do stuff with them would be shorter and you wouldn't feel the need for all those annoying
m# functions.)-
ajax_object returns false rather than throwing an error if AJAX isn't supported. Moreover, it's more complicated than it needs to be. Every browser that matters now supports XMLHttpRequest; the two ActiveXObjects are allowances for IE6, which IMO can safely be ignored. The only ones still stuck with IE6 are companies with pigheaded IT departments (which would probably blacklist your site anyway as "not work related") and people in China (but can they even get to non-whitelisted sites? The thought police seem really strict there). But even if you wanted to support them, it can be simpler -- you can define XMLHttpRequest yourself.Observe:
if (!window.XMLHttpRequest) {
XMLHttpRequest = function() {
// try getting the newest version first
// if that fails, return a mostly-harmless fallback
// if *that* fails, MSXML2 isn't supported and an error is thrown
try {
return new ActiveXObject("Msxml2.XMLHTTP.6.0");
}
catch (ex) {
return new ActiveXObject("Msxml2.XMLHTTP");
}
};
}
// delete function ajax_object altogether; you don't need it anymore
function ajax(path,param,ajax_func,html_div) {
var object=new XMLHttpRequest();
...IE6 includes MSXML v3, as does every OS since Win2k SP4. And v2, i believe, is supported as far back as IE5 and Win98. If your user is running something older than that, stuff should break. Gleefully. And
Microsoft.XMLHTTP is just an alias for Msxml2.XMLHTTP, so you don't really gain anything by trying to use that name as a fallback.-
This "aml" stuff kinda creeps me out. If you intend for the response to be interpreted by JS, a more standard solution like JSON may be a better way to go. It's more portable (namely, you end up with built-in encoding/decoding in both JS and PHP), and you don't need to call a function like
check_aml. In this case, it could be something like var response = JSON.parse(server_response_text); if (response.status == 'fail') {...}, and you could get rid of aml_status_type and check_aml. Though ideally, you'd parse the response in your onreadystatechange handler and pass that to the various functions.-
In
ajax_bookmark, you have code likeif(aml_status===aml_status_type.pass)
{;}
else if (aml_status===aml_status_type.fail)
{;}
else if (aml_status===aml_status_type.undefined)
{The first two if sections should not exist, if you're not going to have anything in them. Only test for the conditions you actually intend to do something about. (If you actually had code there and took it out to lessen the amount of code you paste here, ok. Ignore the suggestion to remove. But for future reference, when you omit stuff, a comment like
/ do stuff here / or an ellipsis (...) helps us know that.)-
document.f1_1.submit();? You use document.getElementById('...') everywhere else; this is inconsistent. Either way, though, you're hard-coding the form name/id, which is kinda...eh. -
I see references to
m2() and m6(). These names are useless; without the code to the functions, there's no way to say what they do. The names should be descriptive enough that one can come in without prior knowledge, read each function individually and get the gist of it, without having to know intimate details about every other function in the script. As it is, i'd have to yoyo through the file (if there were even definitions for the functions, which there aren't in this sample).Code Snippets
if (!window.XMLHttpRequest) {
XMLHttpRequest = function() {
// try getting the newest version first
// if that fails, return a mostly-harmless fallback
// if *that* fails, MSXML2 isn't supported and an error is thrown
try {
return new ActiveXObject("Msxml2.XMLHTTP.6.0");
}
catch (ex) {
return new ActiveXObject("Msxml2.XMLHTTP");
}
};
}
// delete function ajax_object altogether; you don't need it anymore
function ajax(path,param,ajax_func,html_div) {
var object=new XMLHttpRequest();
...if(aml_status===aml_status_type.pass)
{;}
else if (aml_status===aml_status_type.fail)
{;}
else if (aml_status===aml_status_type.undefined)
{Context
StackExchange Code Review Q#5939, answer score: 4
Revisions (0)
No revisions yet.