Skip to content

Commit ddb6cf4

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

File tree

1 file changed

+52
-54
lines changed

1 file changed

+52
-54
lines changed

‎binding.cc‎

Lines changed: 52 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,13 @@ 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+
auto str = std::string(toCh_, toSz_);
101+
delete [] toCh_;
102+
return str;
103+
}
104+
97105
/**
98106
* Create an error object.
99107
*/
@@ -253,19 +261,16 @@ static size_t StringOrBufferLength (napi_env env, napi_value value) {
253261
* Takes a Buffer or string property 'name' from 'opts'.
254262
* Returns null if the property does not exist or is zero-length.
255263
*/
256-
static std::string* RangeOption (napi_env env, napi_value opts, const char* name) {
264+
static std::optional<std::string> RangeOption (napi_env env, napi_value opts, const char* name) {
257265
if (HasProperty(env, opts, name)) {
258266
napi_value value = GetProperty(env, opts, name);
259267

260268
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;
269+
return toString(env, value);
265270
}
266271
}
267272

268-
return NULL;
273+
return {};
269274
}
270275

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

284289
if (napi_get_element(env, arr, i, &element) == napi_ok &&
285290
StringOrBufferLength(env, element) >= 0) {
286-
LD_STRING_OR_BUFFER_TO_COPY(env, element, to);
287-
result->emplace_back(toCh_, toSz_);
288-
delete [] toCh_;
291+
result->push_back(toString(env, element));
289292
}
290293
}
291294
}
@@ -625,20 +628,20 @@ struct PriorityWorker : public BaseWorker {
625628
struct BaseIterator {
626629
BaseIterator(Database* database,
627630
const bool reverse,
628-
std::string* lt,
629-
std::string* lte,
630-
std::string* gt,
631-
std::string* gte,
631+
std::optional<std::string> lt,
632+
std::optional<std::string> lte,
633+
std::optional<std::string> gt,
634+
std::optional<std::string> gte,
632635
const int limit,
633636
const bool fillCache)
634637
: database_(database),
635638
hasClosed_(false),
636639
didSeek_(false),
637640
reverse_(reverse),
638-
lt_(lt),
639-
lte_(lte),
640-
gt_(gt),
641-
gte_(gte),
641+
lt_(std::move(lt)),
642+
lte_(std::move(lte)),
643+
gt_(std::move(gt)),
644+
gte_(std::move(gte)),
642645
limit_(limit),
643646
count_(0) {
644647
options_ = new leveldb::ReadOptions();
@@ -650,11 +653,6 @@ struct BaseIterator {
650653
virtual ~BaseIterator () {
651654
assert(hasClosed_);
652655

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-
658656
delete options_;
659657
}
660658

@@ -668,23 +666,23 @@ struct BaseIterator {
668666
void SeekToRange () {
669667
didSeek_ = true;
670668

671-
if (!reverse_ && gte_ != NULL) {
669+
if (!reverse_ && gte_) {
672670
dbIterator_->Seek(*gte_);
673-
} else if (!reverse_ && gt_ != NULL) {
671+
} else if (!reverse_ && gt_) {
674672
dbIterator_->Seek(*gt_);
675673

676674
if (dbIterator_->Valid() && dbIterator_->key().compare(*gt_) == 0) {
677675
dbIterator_->Next();
678676
}
679-
} else if (reverse_ && lte_ != NULL) {
677+
} else if (reverse_ && lte_) {
680678
dbIterator_->Seek(*lte_);
681679

682680
if (!dbIterator_->Valid()) {
683681
dbIterator_->SeekToLast();
684682
} else if (dbIterator_->key().compare(*lte_) > 0) {
685683
dbIterator_->Prev();
686684
}
687-
} else if (reverse_ && lt_ != NULL) {
685+
} else if (reverse_ && lt_) {
688686
dbIterator_->Seek(*lt_);
689687

690688
if (!dbIterator_->Valid()) {
@@ -784,15 +782,15 @@ struct BaseIterator {
784782
// }
785783

786784
// The lte and gte options take precedence over lt and gt respectively
787-
if (lte_ != NULL) {
785+
if (lte_) {
788786
if (target.compare(*lte_) > 0) return true;
789-
} else if (lt_ != NULL) {
787+
} else if (lt_) {
790788
if (target.compare(*lt_) >= 0) return true;
791789
}
792790

793-
if (gte_ != NULL) {
791+
if (gte_) {
794792
if (target.compare(*gte_) < 0) return true;
795-
} else if (gt_ != NULL) {
793+
} else if (gt_) {
796794
if (target.compare(*gt_) <= 0) return true;
797795
}
798796

@@ -806,10 +804,10 @@ struct BaseIterator {
806804
leveldb::Iterator* dbIterator_;
807805
bool didSeek_;
808806
const bool reverse_;
809-
std::string* lt_;
810-
std::string* lte_;
811-
std::string* gt_;
812-
std::string* gte_;
807+
std::optional<std::string> lt_;
808+
std::optional<std::string> lte_;
809+
std::optional<std::string> gt_;
810+
std::optional<std::string> gte_;
813811
const int limit_;
814812
int count_;
815813
leveldb::ReadOptions* options_;
@@ -825,15 +823,15 @@ struct Iterator final : public BaseIterator {
825823
const bool keys,
826824
const bool values,
827825
const int limit,
828-
std::string* lt,
829-
std::string* lte,
830-
std::string* gt,
831-
std::string* gte,
826+
std::optional<std::string> lt,
827+
std::optional<std::string> lte,
828+
std::optional<std::string> gt,
829+
std::optional<std::string> gte,
832830
const bool fillCache,
833831
const bool keyAsBuffer,
834832
const bool valueAsBuffer,
835833
const uint32_t highWaterMarkBytes)
836-
: BaseIterator(database, reverse, lt, lte, gt, gte, limit, fillCache),
834+
: BaseIterator(database, reverse, std::move(lt), std::move(lte), std::move(gt), std::move(gte), limit, fillCache),
837835
id_(id),
838836
keys_(keys),
839837
values_(values),
@@ -1345,12 +1343,12 @@ struct ClearWorker final : public PriorityWorker {
13451343
napi_value callback,
13461344
const bool reverse,
13471345
const int limit,
1348-
std::string* lt,
1349-
std::string* lte,
1350-
std::string* gt,
1351-
std::string* gte)
1346+
std::optional<std::string> lt,
1347+
std::optional<std::string> lte,
1348+
std::optional<std::string> gt,
1349+
std::optional<std::string> gte)
13521350
: PriorityWorker(env, database, callback, "classic_level.db.clear") {
1353-
iterator_ = new BaseIterator(database, reverse, lt, lte, gt, gte, limit, false);
1351+
iterator_ = new BaseIterator(database, reverse, std::move(lt), std::move(lte), std::move(gt), std::move(gte), limit, false);
13541352
writeOptions_ = new leveldb::WriteOptions();
13551353
writeOptions_->sync = false;
13561354
}
@@ -1409,12 +1407,12 @@ NAPI_METHOD(db_clear) {
14091407
const bool reverse = BooleanProperty(env, options, "reverse", false);
14101408
const int limit = Int32Property(env, options, "limit", -1);
14111409

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");
1410+
const auto lt = RangeOption(env, options, "lt");
1411+
const auto lte = RangeOption(env, options, "lte");
1412+
const auto gt = RangeOption(env, options, "gt");
1413+
const auto gte = RangeOption(env, options, "gte");
14161414

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

14201418
NAPI_RETURN_UNDEFINED();
@@ -1635,14 +1633,14 @@ NAPI_METHOD(iterator_init) {
16351633
const int limit = Int32Property(env, options, "limit", -1);
16361634
const uint32_t highWaterMarkBytes = Uint32Property(env, options, "highWaterMarkBytes", 16 * 1024);
16371635

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");
1636+
auto lt = RangeOption(env, options, "lt");
1637+
auto lte = RangeOption(env, options, "lte");
1638+
auto gt = RangeOption(env, options, "gt");
1639+
auto gte = RangeOption(env, options, "gte");
16421640

16431641
const uint32_t id = database->currentIteratorId_++;
16441642
Iterator* iterator = new Iterator(database, id, reverse, keys,
1645-
values, limit, lt, lte, gt, gte, fillCache,
1643+
values, limit, std::move(lt), std::move(lte), std::move(gt), std::move(gte), fillCache,
16461644
keyAsBuffer, valueAsBuffer, highWaterMarkBytes);
16471645
napi_value result;
16481646

0 commit comments

Comments
 (0)