3
\$\begingroup\$

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:

<Users>
    <Users>
        <UserEmail>jchen@contoso.com</UserEmail>
        <UserPassword>
            BA56E5E0366D003E98EA1C7F04ABF8FCB3753889
        </UserPassword>
    </Users>
    <Users>
        <UserEmail>Kim@contoso.com</UserEmail>
        <UserPassword>
            07B7F3EE06F278DB966BE960E7CBBD103DF30CA6
        </UserPassword>
    </Users>
</Users>

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;
    };
}
\$\endgroup\$

migrated from stackoverflow.com Jun 24 '12 at 16:28

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ It can usefully handle very simple XMLs. It doesn't retrieve the value of attributes. It gives worthless values for element nodes. It doesn't support a decent selector engine. If it's good for your purposes, use it. \$\endgroup\$ – MaxArt Jun 23 '12 at 12:59
  • \$\begingroup\$ "Would you write your own XML Parser? Only if you're f***ing crazy." secretgeek.net/csv_trouble.asp \$\endgroup\$ – David East Jun 23 '12 at 13:33
  • \$\begingroup\$ @David pmpl nice one ;) \$\endgroup\$ – Babibu Jun 23 '12 at 13:40
2
\$\begingroup\$

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

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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