review feedback: early returns and constants for nodeTypes

TooTallNate · Feb 19, 2017 · 7619537 · 7619537
1 parent 85d11c4
commit 7619537
Showing 1 changed file with 58 additions and 50 deletions.
diff --git a/lib/parse.js b/lib/parse.js
@@ -10,6 +10,11 @@ var DOMParser = require('xmldom').DOMParser;
 
 exports.parse = parse;
 
+var TEXT_NODE = 3;
+var CDATA_NODE = 4;
+var COMMENT_NODE = 8;
+
+
 /**
  * We ignore raw text (usually whitespace), <!-- xml comments -->,
  * and raw CDATA nodes.
@@ -20,9 +25,9 @@ exports.parse = parse;
  */
 
 function shouldIgnoreNode (node) {
-  return node.nodeType === 3 // text
-    || node.nodeType === 8   // comment
-    || node.nodeType === 4;  // cdata
+  return node.nodeType === TEXT_NODE
+    || node.nodeType === COMMENT_NODE
+    || node.nodeType === CDATA_NODE;
 }
 
 /**
@@ -88,39 +93,40 @@ function parsePlistXML (node) {
 
   if (node.nodeName === 'plist') {
     new_arr = [];
-    if (!isEmptyNode(node)) {
-      for (i=0; i < node.childNodes.length; i++) {
-        if (!shouldIgnoreNode(node.childNodes[i])) {
-          new_arr.push( parsePlistXML(node.childNodes[i]));
-        }
+    if (isEmptyNode(node)) {
+      return new_arr;
+    }
+    for (i=0; i < node.childNodes.length; i++) {
+      if (!shouldIgnoreNode(node.childNodes[i])) {
+        new_arr.push( parsePlistXML(node.childNodes[i]));
       }
     }
     return new_arr;
-
   } else if (node.nodeName === 'dict') {
     new_obj = {};
     key = null;
     counter = 0;
-    if (!isEmptyNode(node)) {
-      for (i=0; i < node.childNodes.length; i++) {
-        if (shouldIgnoreNode(node.childNodes[i])) continue;
-        if (counter % 2 === 0) {
-          invariant(
-            node.childNodes[i].nodeName === 'key',
-            'Missing key while parsing <dict/>.'
-          );
-          key = parsePlistXML(node.childNodes[i]);
-        } else {
-          invariant(
-            node.childNodes[i].nodeName !== 'key',
-            'Unexpected key "'
-              + parsePlistXML(node.childNodes[i])
-              + '" while parsing <dict/>.'
-          );
-          new_obj[key] = parsePlistXML(node.childNodes[i]);
-        }
-        counter += 1;
+    if (isEmptyNode(node)) {
+      return new_obj;
+    }
+    for (i=0; i < node.childNodes.length; i++) {
+      if (shouldIgnoreNode(node.childNodes[i])) continue;
+      if (counter % 2 === 0) {
+        invariant(
+          node.childNodes[i].nodeName === 'key',
+          'Missing key while parsing <dict/>.'
+        );
+        key = parsePlistXML(node.childNodes[i]);
+      } else {
+        invariant(
+          node.childNodes[i].nodeName !== 'key',
+          'Unexpected key "'
+            + parsePlistXML(node.childNodes[i])
+            + '" while parsing <dict/>.'
+        );
+        new_obj[key] = parsePlistXML(node.childNodes[i]);
       }
+      counter += 1;
     }
     if (counter % 2 === 1) {
       throw new Error('Missing value for "' + key + '" while parsing <dict/>');
@@ -129,12 +135,13 @@ function parsePlistXML (node) {
 
   } else if (node.nodeName === 'array') {
     new_arr = [];
-    if (!isEmptyNode(node)) {
-      for (i=0; i < node.childNodes.length; i++) {
-        if (!shouldIgnoreNode(node.childNodes[i])) {
-          res = parsePlistXML(node.childNodes[i]);
-          if (null != res) new_arr.push(res);
-        }
+    if (isEmptyNode(node)) {
+      return new_arr;
+    }
+    for (i=0; i < node.childNodes.length; i++) {
+      if (!shouldIgnoreNode(node.childNodes[i])) {
+        res = parsePlistXML(node.childNodes[i]);
+        if (null != res) new_arr.push(res);
       }
     }
     return new_arr;
@@ -143,19 +150,19 @@ function parsePlistXML (node) {
     // TODO: what should we do with text types? (CDATA sections)
 
   } else if (node.nodeName === 'key') {
-    res = '';
-    if(!isEmptyNode(node)) {
-      res = node.childNodes[0].nodeValue;
+    if (isEmptyNode(node)) {
+      return '';
     }
-    return res;
+    return node.childNodes[0].nodeValue;
   } else if (node.nodeName === 'string') {
     res = '';
-    if(!isEmptyNode(node)) {
-      for (i=0; i < node.childNodes.length; i++) {
-        var type = node.childNodes[i].nodeType;
-        if (type === 3 || type === 4) {
-          res += node.childNodes[i].nodeValue;
-        }
+    if (isEmptyNode(node)) {
+      return res;
+    }
+    for (i=0; i < node.childNodes.length; i++) {
+      var type = node.childNodes[i].nodeType;
+      if (type === TEXT_NODE || type === CDATA_NODE) {
+        res += node.childNodes[i].nodeValue;
       }
     }
     return res;
@@ -174,19 +181,20 @@ function parsePlistXML (node) {
     );
     res = '';
     for (i=0; i < node.childNodes.length; i++) {
-      if (node.childNodes[i].nodeType === 3) {
+      if (node.childNodes[i].nodeType === TEXT_NODE) {
         res += node.childNodes[i].nodeValue;
       }
     }
     return parseFloat(res);
 
   } else if (node.nodeName === 'data') {
     res = '';
-    if (!isEmptyNode(node)) {
-      for (i=0; i < node.childNodes.length; i++) {
-        if (node.childNodes[i].nodeType === 3) {
-          res += node.childNodes[i].nodeValue.replace(/\s+/g, '');
-        }
+    if (isEmptyNode(node)) {
+      return new Buffer(res, 'base64');
+    }
+    for (i=0; i < node.childNodes.length; i++) {
+      if (node.childNodes[i].nodeType === TEXT_NODE) {
+        res += node.childNodes[i].nodeValue.replace(/\s+/g, '');
       }
     }
     return new Buffer(res, 'base64');