Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I made this function to act sort of like jQuery's $.ajax. I intend to use it in small projects where I do not use jQuery (I don't want to load a whole framework just to facilitate http requests).

Edit: New script incorporating Bob Lauer's suggestions

"use strict";

function OM(obj){
    return new ObjectManipulator(obj);
}
function ObjectManipulator(obj){
    this[0] = obj;
}
ObjectManipulator.prototype.populate = function(newObject){
    for(var i in newObject){
        if(!this[0].hasOwnProperty(i)){
            this[0][i] = newObject[i];
        }
    };
    return this;
}

function ajax(settings){
    return new AjaxRequest(settings);
}
function AjaxRequest(settings){
    settings = OM(settings).populate({
        url:    '/',
        method: 'get',
        type:   'text',
        async:  true,
        data: null
    })[0];

    var done, progress, fail, always;

    var method = settings.method.toUpperCase();
    if(['GET', 'POST', 'PUT', 'DELETE'].indexOf(method) === -1){
        console.log(method + " is not a supported method");
        return;
    }

    var request = new XMLHttpRequest();

    var data = null;
    if(settings.data){
        if(typeof settings.data === 'object'){
            data = new FormData();
            for(prop in settings.data){
                if(settings.data.hasOwnProperty(prop)){
                    data.append(prop, settings.data[prop]);
                }
            }
        }else{
            data = settings.data;
        }
    }

    request.addEventListener('readystatechange', readystatechangeHandler);
    request.addEventListener('progress', progressHandler);

    request.open(method, settings.url, settings.async);
    request.send(data);

    function readystatechangeHandler(){
        if(request.status >= 400){
            if(typeof fail === 'function')   fail(request);
            if(typeof always === 'function') always(request);

            if(typeof fail !== 'function' && typeof always !== 'function'){
                console.error("Error: ", request.status, request.statusText);
                console.error(request);
            }
            request.removeEventListener('readystatechange', readystatechangeHandler);
        }

        if(request.readyState == 4 && request.status == 200){
            if(typeof done === 'function' || typeof always === 'function'){
                var data = request.response;
                if(settings.type.match(/json/gi) || request.responseURL.split(/\./g).pop() == 'json'){
                    data = JSON.parse(request.response);
                }
                if(typeof done == 'function')   done(data);
                if(typeof always == 'function') always(data);
            }
        }
    }
    function progressHandler(event){
        if(!event.lengthComputable) return;
        if(typeof progress == 'function') progress(event);
    }
    this.done     = function(fn){
        done = fn;
        return this;
    }
    this.progress = function(fn){
        progress = fn;
        return this;
    }
    this.fail     = function(fn){
        fail = fn;
        return this;
    }
    this.always   = function(fn){
        always = fn;
        return this;
    }
}

Old script

"use strict";

Object.prototype.populate = function(obj) {
    for(var i in this){
        if(this.hasOwnProperty(i)){
            obj[i] = this[i];
        }
    }
    return obj;
};

function ajax(settings){
    return new ajaxFac(settings);
}
function ajaxFac(settings){
    settings = settings.populate({
        url:    '/',
        method: 'get',
        type:   'text',
        async:  true,
        data: null
    });

    var done, progress, fail, always;

    var method = settings.method.toUpperCase();
    if(['GET', 'POST', 'PUT', 'DELETE'].indexOf(method) == -1){
        console.log(method + " is not a supported method");
        return;
    }

    var request = new XMLHttpRequest();

    var data = null;
    if(settings.data){
        if(typeof settings.data == 'object'){
            data = new FormData();
            for(prop in settings.data){
                if(settings.data.hasOwnProperty(prop)){
                    data.append(prop, settings.data[prop]);
                }
            }
        }else{
            data = settings.data;
        }
    }

    request.addEventListener('readystatechange', readystatechangeHandler);
    request.addEventListener('progress', progressHandler);

    request.open(method, settings.url, settings.async);
    request.send(data);

    function readystatechangeHandler(){
        if(request.status >= 400){
            if(typeof fail == 'function')   fail(request);
            if(typeof always == 'function') always(request);

            if(typeof fail !== 'function' && typeof always !== 'function'){
                console.error("Error: ", request.status, request.statusText);
                console.error(request);
            }
            request.removeEventListener('readystatechange', readystatechangeHandler);
        }

        if(request.readyState == 4 && request.status == 200){
            if(typeof done == 'function' || typeof always == 'function'){
                var data = request.response;
                if(settings.type.match(/json/gi) || request.responseURL.split(/\./g).pop() == 'json'){
                    data = JSON.parse(request.response);
                }
                if(typeof done == 'function')   done(data);
                if(typeof always == 'function') always(data);
            }
        }
    }
    function progressHandler(event){
        if(!event.lengthComputable) return;
        if(typeof progress == 'function') progress(event);
    }

    this.done = function(fn){
        done = fn;
        return this;
    }
    this.progress = function(fn){
        progress = fn;
        return this;
    }
    this.fail = function(fn){
        fail = fn;
        return this;
    }
    this.always = function(fn){
        always = fn;
        return this;
    }
}
share|improve this question

1 Answer 1

up vote 2 down vote accepted
  • Don't extend Object.prototype. Just create a utility function that takes in 2 objects, and copy the values from one to the other.
  • Use ===, instead of ==
  • ajaxFac is not a factory, but a class. Your ajax method is the factory, since its job is to create a new ajaxFac every time. As such, you should rename ajaxFac to something more appropriate, and start its name with a capital letter, to denote that it's instantiated using the new keyword.
  • instead of defining this.done, this.progress, etc. inside the ajaxFunc constructor, define them as ajaxFunc.prototype.done, etc. How you have it set up now, every time you create a new ajaxFunc, a new done, progress, etc. method is created, which takes up memory. If you move those methods to ajaxFunc.prototype, they'll behave exactly the same, but only one method will be created and then shared among all of your ajaxFunc instances. You'll have to structure them slightly different, because var done, progress, fail, always; will no longer be in scope, but you can just as easily do something like this.doneFn = fn; return this;

Edit:

For prototype usage, it should look something like this:

AjaxRequest.prototype.fail = function(fn) {
  this.failFn = fn;
  return this;
};

The tricky part is that inside of your readystatechangeHandler method, this won't refer to your instance of AjaxRequest, because it's an anonymous function. So you'll want to save a reference to this in another variable outside of readystatechangeHandler, and then you can use that. Ex:

var self = this;
function readystatechangeHandler(){
    if(request.status >= 400){
        if(typeof fail === 'function')   self.failFn(request);
        ...
share|improve this answer
    
Thanks for this. These are excellent suggestions. I cringed when I wrote Object.prototype. The only reason I did was because I am planning on making a class to manipulate objects. I have started it with only the populate method. –  Marcel Apr 6 at 22:10
    
I was not able to get the prototyping working for done, progress, etc. Can you give an example for one please? –  Marcel Apr 6 at 22:11
    
I edited my original comment to expand on using prototyping. –  Bob Lauer Apr 7 at 15:22

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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