patternjavascriptMinor
Javascript XML Parser wrapper
Viewed 0 times
javascriptparserwrapperxml
Problem
I created an XML wrapper to easily access XML data.
Please tell me what do you think about it.
This is how you use it:
XML source file:
Source:
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.comXML 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 :
-
-
I would allow access to
-
-
I would
-
You are not checking for falsey values of
-
I would not create a
-
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].dataContext
StackExchange Code Review Q#13014, answer score: 2
Revisions (0)
No revisions yet.