HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Javascript XML Parser wrapper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptparserwrapperxml

Problem

I created an XML wrapper to easily access XML data.
Please tell me what do you think about it.

  • Performance



  • Scalability



  • Anything else...



This is how you use it:

var xml = new Xml(dataString);
xml.load("UserEmail");
alert(xml.length + ", " + xml.getValueAt(0)); // Out: 2, jchen@contoso.com


XML source file:


    
        jchen@contoso.com
        
            BA56E5E0366D003E98EA1C7F04ABF8FCB3753889
        
    
    
        Kim@contoso.com
        
            07B7F3EE06F278DB966BE960E7CBBD103DF30CA6
        
    


Source:

function Xml(xmlString) {
    var parser = function() {
        if (typeof window.DOMParser != "undefined") {
            return (new window.DOMParser()).parseFromString(xmlString,
                    "text/xml");
        }
        else if (typeof window.ActiveXObject != "undefined"
                && new window.ActiveXObject("Microsoft.XMLDOM")) {
            var xmlDoc = new window.ActiveXObject("Microsoft.XMLDOM");
            xmlDoc.async = "false";
            xmlDoc.loadXML(xmlString);
            return xmlDoc;
        }
        else {
            throw new Error("XML parser not found");
        }
    };

    var data = parser(xmlString);
    var elements = null;
    this.length = 0;

    this.load = function(nodeName){
        elements = data.documentElement.getElementsByTagName(nodeName);
        this.length = elements.length;
    };

    this.getValueAt = function(index) {
        if(!elements || index >= this.length){
            return null;
        }
        var element = elements.item(index);

        return element.childNodes[0].data;
    };
}

Solution

From a quick read :

-
Xml seems like a bad name for your wrapper, you should consider something like xmlParser ?

-
I would allow access to data and elements by using this instead of var because you wrap so little of the XML parser API

-
this.length seems wrong ( the parser has no length), maybe loadedElementCount, but even that is pretty bad, I would just let the caller use elements.length.

-
I would return elements in this.load, since that is pretty much what the caller would need next.

-
You are not checking for falsey values of nodeName in this.load

-
I would not create a var element in getValueAt, I would just return elements.item(index).childNodes[0].data

Context

StackExchange Code Review Q#13014, answer score: 2

Revisions (0)

No revisions yet.