4
\$\begingroup\$

I built this as a simple example of using raw JS to perform a simple GET request with a data callback.

Suggestions on how this could be improved in any way, while keeping it simple, would be much appreciated.

JS:

/*jslint unparam: true, white: true */

var app = (function() {
    "use strict";

    return {
        ajax: function(url, callback) {

            var xmlhttp = new window.XMLHttpRequest();

            xmlhttp.onreadystatechange = function() {
                var data;

                if (xmlhttp.readyState === 4 && xmlhttp.status === 200) {
                    data = xmlhttp.responseText;

                    if(typeof callback === 'function') {
                        callback(data);
                    }
                }
            };

            xmlhttp.open('GET', url, true);
            xmlhttp.send();
        }
    };

}());

Example usage:

app.ajax('file.json', function(data) {
    data = JSON.parse(data);
    console.log(data.name);
});
\$\endgroup\$
5
  • \$\begingroup\$ you are aware this is not cross-browser compatible? here's something I used when I did something similar \$\endgroup\$
    – Vogel612
    Commented Mar 19, 2014 at 12:08
  • \$\begingroup\$ @Vogel612 Do you mean with regards to IE6/7? \$\endgroup\$ Commented Mar 19, 2014 at 12:14
  • \$\begingroup\$ yea. Don't expect ppl. on public websites to have current browsers. no offense if you don't implement it though ;) It's just a hassle for that minimal helpfulnes. \$\endgroup\$
    – Vogel612
    Commented Mar 19, 2014 at 12:15
  • 1
    \$\begingroup\$ I don't really care that much about IE6/7 support. Thanks for pointing this out anyway. \$\endgroup\$ Commented Mar 19, 2014 at 12:24
  • \$\begingroup\$ I rolled back your edit, it is considered bad form to update this question because it puts answer <> question out of sync. \$\endgroup\$
    – konijn
    Commented Mar 19, 2014 at 13:41

1 Answer 1

3
\$\begingroup\$

From a once over:

  • There should be a way to handle errors, this is only good for sunny day scenarios
  • It would be more useful if you also passed the xmlhttp object to the callback : callback(data, xmlhttp);
  • As stated by Vogel612, it wont work for some versions of IE, it's not hard to add support for those
  • There is no need at all to declare data, you can pass xmlhttp.responseText; straight to callback, it would make the code tighter
  • A comment as to what 4 and 200 stand for could be useful for the unaware reader

Other than that, looks good.

\$\endgroup\$
2
  • \$\begingroup\$ Here at Code Review we don't like magic numbers +1 ! \$\endgroup\$
    – Marc-Andre
    Commented Mar 19, 2014 at 13:04
  • \$\begingroup\$ Thanks for the feedback. I've updated the solution in the original question. \$\endgroup\$ Commented Mar 19, 2014 at 13:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.