Skip to content

Commit 6aecd45

Browse files
committed
use std::optional<std::string> over std::string*
1 parent 081701d commit 6aecd45

File tree

1 file changed

+50
-54
lines changed

1 file changed

+50
-54
lines changed

binding.cc

Lines changed: 50 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <leveldb/filter_policy.h>
1111

1212
#include <map>
13+
#include <optional>
1314
#include <vector>
1415

1516
/**
@@ -94,6 +95,11 @@ static bool IsObject (napi_env env, napi_value value) {
9495
return type == napi_object;
9596
}
9697

98+
std::string toString(napi_env& env, const napi_value& from) {
99+
LD_STRING_OR_BUFFER_TO_COPY(env, from, to);
100+
return std::string(toCh_, toSz_);
101+
}
102+
97103
/**
98104
* Create an error object.
99105
*/
@@ -253,19 +259,16 @@ static size_t StringOrBufferLength (napi_env env, napi_value value) {
253259
* Takes a Buffer or string property 'name' from 'opts'.
254260
* Returns null if the property does not exist or is zero-length.
255261
*/
256-
static std::string* RangeOption (napi_env env, napi_value opts, const char* name) {
262+
static std::optional<std::string> RangeOption (napi_env env, napi_value opts, const char* name) {
257263
if (HasProperty(env, opts, name)) {
258264
napi_value value = GetProperty(env, opts, name);
259265

260266
if (StringOrBufferLength(env, value) >= 0) {
261-
LD_STRING_OR_BUFFER_TO_COPY(env, value, to);
262-
std::string* result = new std::string(toCh_, toSz_);
263-
delete [] toCh_;
264-
return result;
267+
return toString(env, value);
265268
}
266269
}
267270

268-
return NULL;
271+
return {};
269272
}
270273

271274
/**
@@ -283,9 +286,7 @@ static std::vector<std::string>* KeyArray (napi_env env, napi_value arr) {
283286

284287
if (napi_get_element(env, arr, i, &element) == napi_ok &&
285288
StringOrBufferLength(env, element) >= 0) {
286-
LD_STRING_OR_BUFFER_TO_COPY(env, element, to);
287-
result->emplace_back(toCh_, toSz_);
288-
delete [] toCh_;
289+
result->push_back(toString(env, element));
289290
}
290291
}
291292
}
@@ -625,20 +626,20 @@ struct PriorityWorker : public BaseWorker {
625626
struct BaseIterator {
626627
BaseIterator(Database* database,
627628
const bool reverse,
628-
std::string* lt,
629-
std::string* lte,
630-
std::string* gt,
631-
std::string* gte,
629+
std::optional<std::string> lt,
630+
std::optional<std::string> lte,
631+
std::optional<std::string> gt,
632+
std::optional<std::string> gte,
632633
const int limit,
633634
const bool fillCache)
634635
: database_(database),
635636
hasClosed_(false),
636637
didSeek_(false),
637638
reverse_(reverse),
638-
lt_(lt),
639-
lte_(lte),
640-
gt_(gt),
641-
gte_(gte),
639+
lt_(std::move(lt)),
640+
lte_(std::move(lte)),
641+
gt_(std::move(gt)),
642+
gte_(std::move(gte)),
642643
limit_(limit),
643644
count_(0) {
644645
options_ = new leveldb::ReadOptions();
@@ -650,11 +651,6 @@ struct BaseIterator {
650651
virtual ~BaseIterator () {
651652
assert(hasClosed_);
652653

653-
if (lt_ != NULL) delete lt_;
654-
if (gt_ != NULL) delete gt_;
655-
if (lte_ != NULL) delete lte_;
656-
if (gte_ != NULL) delete gte_;
657-
658654
delete options_;
659655
}
660656

@@ -668,23 +664,23 @@ struct BaseIterator {
668664
void SeekToRange () {
669665
didSeek_ = true;
670666

671-
if (!reverse_ && gte_ != NULL) {
667+
if (!reverse_ && gte_) {
672668
dbIterator_->Seek(*gte_);
673-
} else if (!reverse_ && gt_ != NULL) {
669+
} else if (!reverse_ && gt_) {
674670
dbIterator_->Seek(*gt_);
675671

676672
if (dbIterator_->Valid() && dbIterator_->key().compare(*gt_) == 0) {
677673
dbIterator_->Next();
678674
}
679-
} else if (reverse_ && lte_ != NULL) {
675+
} else if (reverse_ && lte_) {
680676
dbIterator_->Seek(*lte_);
681677

682678
if (!dbIterator_->Valid()) {
683679
dbIterator_->SeekToLast();
684680
} else if (dbIterator_->key().compare(*lte_) > 0) {
685681
dbIterator_->Prev();
686682
}
687-
} else if (reverse_ && lt_ != NULL) {
683+
} else if (reverse_ && lt_) {
688684
dbIterator_->Seek(*lt_);
689685

690686
if (!dbIterator_->Valid()) {
@@ -784,15 +780,15 @@ struct BaseIterator {
784780
// }
785781

786782
// The lte and gte options take precedence over lt and gt respectively
787-
if (lte_ != NULL) {
783+
if (lte_) {
788784
if (target.compare(*lte_) > 0) return true;
789-
} else if (lt_ != NULL) {
785+
} else if (lt_) {
790786
if (target.compare(*lt_) >= 0) return true;
791787
}
792788

793-
if (gte_ != NULL) {
789+
if (gte_) {
794790
if (target.compare(*gte_) < 0) return true;
795-
} else if (gt_ != NULL) {
791+
} else if (gt_) {
796792
if (target.compare(*gt_) <= 0) return true;
797793
}
798794

@@ -806,10 +802,10 @@ struct BaseIterator {
806802
leveldb::Iterator* dbIterator_;
807803
bool didSeek_;
808804
const bool reverse_;
809-
std::string* lt_;
810-
std::string* lte_;
811-
std::string* gt_;
812-
std::string* gte_;
805+
std::optional<std::string> lt_;
806+
std::optional<std::string> lte_;
807+
std::optional<std::string> gt_;
808+
std::optional<std::string> gte_;
813809
const int limit_;
814810
int count_;
815811
leveldb::ReadOptions* options_;
@@ -825,15 +821,15 @@ struct Iterator final : public BaseIterator {
825821
const bool keys,
826822
const bool values,
827823
const int limit,
828-
std::string* lt,
829-
std::string* lte,
830-
std::string* gt,
831-
std::string* gte,
824+
std::optional<std::string> lt,
825+
std::optional<std::string> lte,
826+
std::optional<std::string> gt,
827+
std::optional<std::string> gte,
832828
const bool fillCache,
833829
const bool keyAsBuffer,
834830
const bool valueAsBuffer,
835831
const uint32_t highWaterMarkBytes)
836-
: BaseIterator(database, reverse, lt, lte, gt, gte, limit, fillCache),
832+
: BaseIterator(database, reverse, std::move(lt), std::move(lte), std::move(gt), std::move(gte), limit, fillCache),
837833
id_(id),
838834
keys_(keys),
839835
values_(values),
@@ -1345,12 +1341,12 @@ struct ClearWorker final : public PriorityWorker {
13451341
napi_value callback,
13461342
const bool reverse,
13471343
const int limit,
1348-
std::string* lt,
1349-
std::string* lte,
1350-
std::string* gt,
1351-
std::string* gte)
1344+
std::optional<std::string> lt,
1345+
std::optional<std::string> lte,
1346+
std::optional<std::string> gt,
1347+
std::optional<std::string> gte)
13521348
: PriorityWorker(env, database, callback, "classic_level.db.clear") {
1353-
iterator_ = new BaseIterator(database, reverse, lt, lte, gt, gte, limit, false);
1349+
iterator_ = new BaseIterator(database, reverse, std::move(lt), std::move(lte), std::move(gt), std::move(gte), limit, false);
13541350
writeOptions_ = new leveldb::WriteOptions();
13551351
writeOptions_->sync = false;
13561352
}
@@ -1409,12 +1405,12 @@ NAPI_METHOD(db_clear) {
14091405
const bool reverse = BooleanProperty(env, options, "reverse", false);
14101406
const int limit = Int32Property(env, options, "limit", -1);
14111407

1412-
std::string* lt = RangeOption(env, options, "lt");
1413-
std::string* lte = RangeOption(env, options, "lte");
1414-
std::string* gt = RangeOption(env, options, "gt");
1415-
std::string* gte = RangeOption(env, options, "gte");
1408+
const auto lt = RangeOption(env, options, "lt");
1409+
const auto lte = RangeOption(env, options, "lte");
1410+
const auto gt = RangeOption(env, options, "gt");
1411+
const auto gte = RangeOption(env, options, "gte");
14161412

1417-
ClearWorker* worker = new ClearWorker(env, database, callback, reverse, limit, lt, lte, gt, gte);
1413+
ClearWorker* worker = new ClearWorker(env, database, callback, reverse, limit, std::move(lt), std::move(lte), std::move(gt), std::move(gte));
14181414
worker->Queue(env);
14191415

14201416
NAPI_RETURN_UNDEFINED();
@@ -1635,14 +1631,14 @@ NAPI_METHOD(iterator_init) {
16351631
const int limit = Int32Property(env, options, "limit", -1);
16361632
const uint32_t highWaterMarkBytes = Uint32Property(env, options, "highWaterMarkBytes", 16 * 1024);
16371633

1638-
std::string* lt = RangeOption(env, options, "lt");
1639-
std::string* lte = RangeOption(env, options, "lte");
1640-
std::string* gt = RangeOption(env, options, "gt");
1641-
std::string* gte = RangeOption(env, options, "gte");
1634+
auto lt = RangeOption(env, options, "lt");
1635+
auto lte = RangeOption(env, options, "lte");
1636+
auto gt = RangeOption(env, options, "gt");
1637+
auto gte = RangeOption(env, options, "gte");
16421638

16431639
const uint32_t id = database->currentIteratorId_++;
16441640
Iterator* iterator = new Iterator(database, id, reverse, keys,
1645-
values, limit, lt, lte, gt, gte, fillCache,
1641+
values, limit, std::move(lt), std::move(lte), std::move(gt), std::move(gte), fillCache,
16461642
keyAsBuffer, valueAsBuffer, highWaterMarkBytes);
16471643
napi_value result;
16481644

0 commit comments

Comments
 (0)