0

I want to implement some good OOP practices into my javascript somehow. As you can see I am having an abstractAjaxRequest and then giving it some children that define its getAjaxResponse function. I get the error posted at the bottom, on this line: self.getAjaxResponse(). Anyone see the error of my ways?

function soapReq() {
    var ajaxRequest = new SignInAjaxReq();
    ajaxRequest.init();
    var SOAPRequest = getSOAPHead();
    var bodyArgs = getLoginAttempt();
    SOAPRequest += getSOAPBody(bodyArgs[0], bodyArgs[1], bodyArgs[2]);
    SOAPRequest += getSOAPFoot();

    var url = 'xxx'
    ajaxRequest.ajaxRequest.open('POST', url, true);
    ajaxRequest.ajaxRequest.setRequestHeader( "Content-Type","text/xml; charset=utf-8");
    ajaxRequest.ajaxRequest.send(SOAPRequest);
}    

function AbstractAjaxReq() {
    var self = this;
    this.init = function() {
        self.ajaxRequest = new XMLHttpRequest();
        self.ajaxRequest.onreadystatechange = function() {
            if(self.ajaxRequest.readyState === 4){
                self.getAjaxResponse() // error here
            }
        };
        return self.ajaxRequest;
    };
};

function SignInAjaxReq() {
    var self = this;
    this.getAjaxResponse = function() {
        var xml = self.ajaxRequest.responseXML;
        var x=xml.getElementsByTagName("signInResult");
        for (i=0;i<x.length;i++) { 
            console.log(x[i].childNodes[0].nodeValue);
        }
    };
};
SignInAjaxReq.prototype = new AbstractAjaxReq();

Uncaught TypeError: Object #<AbstractAjaxReq> has no method 'getAjaxResponse' SoapRequest.js:42
    self.ajaxRequest.onreadystatechange
RevanProdigalKnight
  • 1,288
  • 1
  • 14
  • 23
btf217
  • 1
  • 2
  • 6
  • what line does the error come from? – agconti Aug 07 '14 at 20:46
  • That is in the question. Thanks. – btf217 Aug 07 '14 at 20:47
  • `this.ajaxRequest.readyState` the `this` is the function scoped `this`, not the AbstractAjaxReq.`this` - if you know what i mean to say :) – birdspider Aug 07 '14 at 20:47
  • `this` doesn't refer to what you think it does http://stackoverflow.com/questions/3127429/javascript-this-keyword – dee-see Aug 07 '14 at 20:50
  • to clarify: you are using `this` in multiple levels of fn(){ fn{} } - if I remeber correctly this is a different `this` every time – birdspider Aug 07 '14 at 20:50
  • @birdspider Thanks! What syntax would I use to correct that? – btf217 Aug 07 '14 at 20:50
  • @btf217 check out Vache's link - js is not oop based but prototype based which is supposedlty very powerfull but different from classic oop – birdspider Aug 07 '14 at 20:52
  • 1
    Javascript is not c# or java. It is actually a functional language that supports prototypical inheritance (which is not the same as object oriented). So what you are doing is not really 'good OOP practices'. Better to stick to javascript best practices IMHO. – Mike Cheel Aug 07 '14 at 20:56
  • @MikeCheel Wouldn't that mean a lot of code duplication in this circumstance? – btf217 Aug 07 '14 at 21:06
  • I didn't say avoid reuse (dry principle). I'm just saying what might be best practices in an OOP world are not necessarily best practices in javascript land. I've seen a lot of problems where people think they can just treat javascript like c# and java. – Mike Cheel Aug 07 '14 at 21:10

1 Answers1

2

Your inheritance is flawed. When you create the prototype by new AbstractReq(), then the self closure variable will point to that prototype object - which does not have a getAjaxResponse method.

Fix #1: Use correct inheritance:

function AbstractAjaxReq() {
    var self = this;
    this.init = function() {
        self.ajaxRequest = new XMLHttpRequest();
        self.ajaxRequest.onreadystatechange = function() {
            if(self.ajaxRequest.readyState === 4){
                self.getAjaxResponse() // no more error here
            }
        };
        return self.ajaxRequest;
    };
};

function SignInAjaxReq() {
    AbstractAjaxReq.call(this); // invoke the super constructor
    var self = this;
    this.getAjaxResponse = function() {
        var xml = self.ajaxRequest.responseXML;
        var x=xml.getElementsByTagName("signInResult");
        for (i=0;i<x.length;i++) { 
            console.log(x[i].childNodes[0].nodeValue);
        }
    };
};
SignInAjaxReq.prototype = Object.create(AbstractAjaxReq.prototype);

Fix #2: Assign the actual instance to self by creating the variable in the init function:

function AbstractAjaxReq() {
    this.init = function() {
        var self = this; // here!
        self.ajaxRequest = new XMLHttpRequest();
        self.ajaxRequest.onreadystatechange = function() {
            if(self.ajaxRequest.readyState === 4){
                self.getAjaxResponse() // no more error here
            }
        };
        return self.ajaxRequest;
    };
};

Also, use the prototype! Example:

var abstractAjaxPrototype = {
    init: function() {
        var self = this; // here!
        this.ajaxRequest = new XMLHttpRequest();
        this.ajaxRequest.onreadystatechange = function() {
            if (self.ajaxRequest.readyState === 4){
                self.getAjaxResponse() // no more error here
            }
        };
        return this.ajaxRequest;
    }
};
function SignInAjaxReq() { }
SignInAjaxReq.prototype = Object.create(abstractAjaxPrototype);
SignInAjaxReq.prototype.getAjaxResponse = function() {
    var xml = this.ajaxRequest.responseXML;
    var x = xml.getElementsByTagName("signInResult");
    for (i=0; i<x.length; i++) { 
        console.log(x[i].childNodes[0].nodeValue);
    }
};

And: don't use init methods, use the constructor:

function AbstractAjaxReq() {
    var self = this; // here!
    self.ajaxRequest = new XMLHttpRequest();
    self.ajaxRequest.onreadystatechange = function() {
        if(self.ajaxRequest.readyState === 4){
            self.getAjaxResponse() // no more error here
        }
    };
};
function SignInAjaxReq() {
    AbstractAjaxReq.call(this); // invoke the super constructor
}
SignInAjaxReq.prototype = Object.create(abstractAjaxPrototype);
SignInAjaxReq.prototype.getAjaxResponse = function() {
    var xml = this.ajaxRequest.responseXML;
    var x = xml.getElementsByTagName("signInResult");
    for (i=0; i<x.length; i++) { 
        console.log(x[i].childNodes[0].nodeValue);
    }
};

// Usage:
var ajaxRequest = new SignInAjaxReq();
// no ajaxRequest.init()!
Community
  • 1
  • 1
Bergi
  • 513,640
  • 108
  • 821
  • 1,164