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);
});
konijn
34.3k5 gold badges70 silver badges267 bronze badges
asked Mar 19, 2014 at 11:25
\$\endgroup\$
5
  • \$\begingroup\$ you are aware this is not cross-browser compatible? here's something I used when I did something similar \$\endgroup\$ 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\$ 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\$ 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.

answered Mar 19, 2014 at 12:40
\$\endgroup\$
2
  • \$\begingroup\$ Here at Code Review we don't like magic numbers +1 ! \$\endgroup\$ 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

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.