Refactor initialization of range options

Cherry-picked from Level/leveldown#681

Author: vweevers

binding.cc

@@ -238,6 +238,25 @@ static size_t StringOrBufferLength (napi_env env, napi_value value) {
   return size;
 }
 
+/**
+ * Takes a Buffer or string property 'name' from 'opts'.
+ * Returns null if the property does not exist or is zero-length.
+ */
+static std::string* RangeOption (napi_env env, napi_value opts, const char* name) {
+  if (HasProperty(env, opts, name)) {
+    napi_value value = GetProperty(env, opts, name);
+
+    if (StringOrBufferLength(env, value) > 0) {
+      LD_STRING_OR_BUFFER_TO_COPY(env, value, to);
+      std::string* result = new std::string(toCh_, toSz_);
+      delete [] toCh_;
+      return result;
+    }
+  }
+
+  return NULL;
+}
+
 /**
  * Calls a function.
  */
@@ -493,7 +512,7 @@ struct PriorityWorker : public BaseWorker {
 struct Iterator {
   Iterator (Database* database,
             uint32_t id,
-            leveldb::Slice* start,
+            std::string* start,
             std::string* end,
             bool reverse,
             bool keys,
@@ -540,32 +559,14 @@ struct Iterator {
   ~Iterator () {
     assert(ended_);
     ReleaseTarget();
-    if (start_ != NULL) {
-      // Special case for `start` option: it won't be
-      // freed up by any of the delete calls below.
-      if (!((lt_ != NULL && reverse_)
-            || (lte_ != NULL && reverse_)
-            || (gt_ != NULL && !reverse_)
-            || (gte_ != NULL && !reverse_))) {
-        delete [] start_->data();
-      }
-      delete start_;
-    }
-    if (end_ != NULL) {
-      delete end_;
-    }
-    if (lt_ != NULL) {
-      delete lt_;
-    }
-    if (gt_ != NULL) {
-      delete gt_;
-    }
-    if (lte_ != NULL) {
-      delete lte_;
-    }
-    if (gte_ != NULL) {
-      delete gte_;
-    }
+
+    if (start_ != NULL) delete start_;
+    if (end_ != NULL) delete end_;
+    if (lt_ != NULL) delete lt_;
+    if (gt_ != NULL) delete gt_;
+    if (lte_ != NULL) delete lte_;
+    if (gte_ != NULL) delete gte_;
+
     delete options_;
   }
 
@@ -730,7 +731,7 @@ struct Iterator {
 
   Database* database_;
   uint32_t id_;
-  leveldb::Slice* start_;
+  std::string* start_;
   std::string* end_;
   bool reverse_;
   bool keys_;
@@ -1302,17 +1303,6 @@ static void FinalizeIterator (napi_env env, void* data, void* hint) {
   }
 }
 
-#define CHECK_PROPERTY(name, code)                                      \
-  if (HasProperty(env, options, #name)) {                               \
-    napi_value value = GetProperty(env, options, #name);                \
-    if (IsString(env, value) || IsBuffer(env, value)) {                 \
-      if (StringOrBufferLength(env, value) > 0) {                       \
-        LD_STRING_OR_BUFFER_TO_COPY(env, value, _##name);               \
-        code;                                                           \
-      }                                                                 \
-    }                                                                   \
-  }                                                                     \
-
 /**
  * Create an iterator.
  */
@@ -1331,84 +1321,18 @@ NAPI_METHOD(iterator_init) {
   uint32_t highWaterMark = Uint32Property(env, options, "highWaterMark",
                                           16 * 1024);
 
-  // TODO simplify and refactor the hideous code below
-
-  leveldb::Slice* start = NULL;
-  char *startStr = NULL;
-  CHECK_PROPERTY(start, {
-    start = new leveldb::Slice(_startCh_, _startSz_);
-    startStr = _startCh_;
-  });
-
-  std::string* end = NULL;
-  CHECK_PROPERTY(end, {
-    end = new std::string(_endCh_, _endSz_);
-    delete [] _endCh_;
-  });
-
-  std::string* lt = NULL;
-  CHECK_PROPERTY(lt, {
-    lt = new std::string(_ltCh_, _ltSz_);
-    delete [] _ltCh_;
-    if (reverse) {
-      if (startStr != NULL) {
-        delete [] startStr;
-        startStr = NULL;
-      }
-      if (start != NULL) {
-        delete start;
-      }
-      start = new leveldb::Slice(lt->data(), lt->size());
-    }
-  });
-
-  std::string* lte = NULL;
-  CHECK_PROPERTY(lte, {
-    lte = new std::string(_lteCh_, _lteSz_);
-    delete [] _lteCh_;
-    if (reverse) {
-      if (startStr != NULL) {
-        delete [] startStr;
-        startStr = NULL;
-      }
-      if (start != NULL) {
-        delete start;
-      }
-      start = new leveldb::Slice(lte->data(), lte->size());
-    }
-  });
-
-  std::string* gt = NULL;
-  CHECK_PROPERTY(gt, {
-    gt = new std::string(_gtCh_, _gtSz_);
-    delete [] _gtCh_;
-    if (!reverse) {
-      if (startStr != NULL) {
-        delete [] startStr;
-        startStr = NULL;
-      }
-      if (start != NULL) {
-        delete start;
-      }
-      start = new leveldb::Slice(gt->data(), gt->size());
-    }
-  });
-
-  std::string* gte = NULL;
-  CHECK_PROPERTY(gte, {
-    gte = new std::string(_gteCh_, _gteSz_);
-    delete [] _gteCh_;
-    if (!reverse) {
-      if (startStr != NULL) {
-        delete [] startStr;
-        startStr = NULL;
-      }
-      if (start != NULL) {
-        delete start;
-      }
-      start = new leveldb::Slice(gte->data(), gte->size());
-    }
-  });
+  std::string* start = NULL;
+  std::string* end = RangeOption(env, options, "end");
+  std::string* lt = RangeOption(env, options, "lt");
+  std::string* lte = RangeOption(env, options, "lte");
+  std::string* gt = RangeOption(env, options, "gt");
+  std::string* gte = RangeOption(env, options, "gte");
+
+  if (!reverse && gte != NULL) start = new std::string(*gte);
+  else if (!reverse && gt != NULL) start = new std::string(*gt);
+  else if (reverse && lte != NULL) start = new std::string(*lte);
+  else if (reverse && lt != NULL) start = new std::string(*lt);
+  else start = RangeOption(env, options, "start");
 
   uint32_t id = database->currentIteratorId_++;
   Iterator* iterator = new Iterator(database, id, start, end, reverse, keys,