Skip to content

Commit cdf78bf

Browse files
zenon8adamsdaniellockyer
zenon8adams
authored and
daniellockyer
committed
- Extensively utilized automatic type deduction for improved code readability
- Implemented RAII to ensure resource safety and exception safety - Leveraged range-based for loops for cleaner and more efficient iteration - Addressed and resolved all project-related warnings for enhanced code quality
1 parent 22ac6cc commit cdf78bf

File tree

9 files changed

+306
-315
lines changed

9 files changed

+306
-315
lines changed

Diff for: ‎src/async.h

+5-6
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,19 @@ template <class Item, class Parent> class Async {
2929
}
3030

3131
static void listener(uv_async_t* handle) {
32-
Async* async = static_cast<Async*>(handle->data);
32+
auto* async = static_cast<Async*>(handle->data);
3333
std::vector<Item*> rows;
3434
NODE_SQLITE3_MUTEX_LOCK(&async->mutex)
3535
rows.swap(async->data);
3636
NODE_SQLITE3_MUTEX_UNLOCK(&async->mutex)
37-
for (unsigned int i = 0, size = rows.size(); i < size; i++) {
38-
async->callback(async->parent, rows[i]);
39-
}
37+
for(auto row : rows)
38+
async->callback(async->parent, row);
4039
}
4140

4241
static void close(uv_handle_t* handle) {
4342
assert(handle != NULL);
4443
assert(handle->data != NULL);
45-
Async* async = static_cast<Async*>(handle->data);
44+
auto* async = static_cast<Async*>(handle->data);
4645
delete async;
4746
}
4847

@@ -56,7 +55,7 @@ template <class Item, class Parent> class Async {
5655

5756
void add(Item* item) {
5857
NODE_SQLITE3_MUTEX_LOCK(&mutex);
59-
data.push_back(item);
58+
data.emplace_back(item);
6059
NODE_SQLITE3_MUTEX_UNLOCK(&mutex)
6160
}
6261

Diff for: ‎src/backup.cc

+46-47
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
#include <string.h>
1+
#include <cstring>
22
#include <napi.h>
3-
43
#include "macros.h"
54
#include "database.h"
65
#include "backup.h"
@@ -11,9 +10,9 @@ Napi::Object Backup::Init(Napi::Env env, Napi::Object exports) {
1110
Napi::HandleScope scope(env);
1211

1312
// declare napi_default_method here as it is only available in Node v14.12.0+
14-
napi_property_attributes napi_default_method = static_cast<napi_property_attributes>(napi_writable | napi_configurable);
13+
auto napi_default_method = static_cast<napi_property_attributes>(napi_writable | napi_configurable);
1514

16-
Napi::Function t = DefineClass(env, "Backup", {
15+
auto t = DefineClass(env, "Backup", {
1716
InstanceMethod("step", &Backup::Step, napi_default_method),
1817
InstanceMethod("finish", &Backup::Finish, napi_default_method),
1918
InstanceAccessor("idle", &Backup::IdleGetter, nullptr),
@@ -34,7 +33,7 @@ void Backup::Process() {
3433
}
3534

3635
while (inited && !locked && !queue.empty()) {
37-
std::unique_ptr<Call> call(queue.front());
36+
auto call = std::move(queue.front());
3837
queue.pop();
3938

4039
call->callback(call->baton);
@@ -43,19 +42,19 @@ void Backup::Process() {
4342

4443
void Backup::Schedule(Work_Callback callback, Baton* baton) {
4544
if (finished) {
46-
queue.push(new Call(callback, baton));
45+
queue.emplace(new Call(callback, baton));
4746
CleanQueue();
4847
}
4948
else if (!inited || locked || !queue.empty()) {
50-
queue.push(new Call(callback, baton));
49+
queue.emplace(new Call(callback, baton));
5150
}
5251
else {
5352
callback(baton);
5453
}
5554
}
5655

5756
template <class T> void Backup::Error(T* baton) {
58-
Napi::Env env = baton->backup->Env();
57+
auto env = baton->backup->Env();
5958
Napi::HandleScope scope(env);
6059

6160
Backup* backup = baton->backup;
@@ -76,7 +75,7 @@ template <class T> void Backup::Error(T* baton) {
7675
}
7776

7877
void Backup::CleanQueue() {
79-
Napi::Env env = this->Env();
78+
auto env = this->Env();
8079
Napi::HandleScope scope(env);
8180

8281
if (inited && !queue.empty()) {
@@ -88,7 +87,7 @@ void Backup::CleanQueue() {
8887

8988
// Clear out the queue so that this object can get GC'ed.
9089
while (!queue.empty()) {
91-
std::unique_ptr<Call> call(queue.front());
90+
auto call = std::move(queue.front());
9291
queue.pop();
9392

9493
std::unique_ptr<Baton> baton(call->baton);
@@ -111,7 +110,7 @@ void Backup::CleanQueue() {
111110
else while (!queue.empty()) {
112111
// Just delete all items in the queue; we already fired an event when
113112
// initializing the backup failed.
114-
std::unique_ptr<Call> call(queue.front());
113+
auto call = std::move(queue.front());
115114
queue.pop();
116115

117116
// We don't call the actual callback, so we have to make sure that
@@ -121,13 +120,13 @@ void Backup::CleanQueue() {
121120
}
122121

123122
Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info) {
124-
Napi::Env env = info.Env();
123+
auto env = info.Env();
125124
if (!info.IsConstructCall()) {
126125
Napi::TypeError::New(env, "Use the new operator to create new Backup objects").ThrowAsJavaScriptException();
127126
return;
128127
}
129128

130-
int length = info.Length();
129+
auto length = info.Length();
131130

132131
if (length <= 0 || !Database::HasInstance(info[0])) {
133132
Napi::TypeError::New(env, "Database object expected").ThrowAsJavaScriptException();
@@ -154,32 +153,32 @@ Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info)
154153
return;
155154
}
156155

157-
Database* db = Napi::ObjectWrap<Database>::Unwrap(info[0].As<Napi::Object>());
158-
Napi::String filename = info[1].As<Napi::String>();
159-
Napi::String sourceName = info[2].As<Napi::String>();
160-
Napi::String destName = info[3].As<Napi::String>();
161-
Napi::Boolean filenameIsDest = info[4].As<Napi::Boolean>();
156+
auto* database = Napi::ObjectWrap<Database>::Unwrap(info[0].As<Napi::Object>());
157+
auto filename = info[1].As<Napi::String>();
158+
auto sourceName = info[2].As<Napi::String>();
159+
auto destName = info[3].As<Napi::String>();
160+
auto filenameIsDest = info[4].As<Napi::Boolean>();
162161

163162
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("filename", filename));
164163
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("sourceName", sourceName));
165164
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("destName", destName));
166165
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("filenameIsDest", filenameIsDest));
167166

168-
init(db);
167+
init(database);
169168

170-
InitializeBaton* baton = new InitializeBaton(db, info[5].As<Napi::Function>(), this);
169+
auto* baton = new InitializeBaton(database, info[5].As<Napi::Function>(), this);
171170
baton->filename = filename.Utf8Value();
172171
baton->sourceName = sourceName.Utf8Value();
173172
baton->destName = destName.Utf8Value();
174173
baton->filenameIsDest = filenameIsDest.Value();
175-
db->Schedule(Work_BeginInitialize, baton);
174+
database->Schedule(Work_BeginInitialize, baton);
176175
}
177176

178177
void Backup::Work_BeginInitialize(Database::Baton* baton) {
179178
assert(baton->db->open);
180179
baton->db->pending++;
181-
Napi::Env env = baton->db->Env();
182-
int status = napi_create_async_work(
180+
auto env = baton->db->Env();
181+
int UNUSED(status) = napi_create_async_work(
183182
env, NULL, Napi::String::New(env, "sqlite3.Backup.Initialize"),
184183
Work_Initialize, Work_AfterInitialize, baton, &baton->request
185184
);
@@ -192,7 +191,7 @@ void Backup::Work_Initialize(napi_env e, void* data) {
192191

193192
// In case stepping fails, we use a mutex to make sure we get the associated
194193
// error message.
195-
sqlite3_mutex* mtx = sqlite3_db_mutex(baton->db->_handle);
194+
auto* mtx = sqlite3_db_mutex(baton->db->_handle);
196195
sqlite3_mutex_enter(mtx);
197196

198197
backup->status = sqlite3_open(baton->filename.c_str(), &backup->_otherDb);
@@ -218,9 +217,9 @@ void Backup::Work_Initialize(napi_env e, void* data) {
218217

219218
void Backup::Work_AfterInitialize(napi_env e, napi_status status, void* data) {
220219
std::unique_ptr<InitializeBaton> baton(static_cast<InitializeBaton*>(data));
221-
Backup* backup = baton->backup;
220+
auto* backup = baton->backup;
222221

223-
Napi::Env env = backup->Env();
222+
auto env = backup->Env();
224223
Napi::HandleScope scope(env);
225224

226225
if (backup->status != SQLITE_OK) {
@@ -239,13 +238,13 @@ void Backup::Work_AfterInitialize(napi_env e, napi_status status, void* data) {
239238
}
240239

241240
Napi::Value Backup::Step(const Napi::CallbackInfo& info) {
242-
Backup* backup = this;
243-
Napi::Env env = backup->Env();
241+
auto* backup = this;
242+
auto env = backup->Env();
244243

245244
REQUIRE_ARGUMENT_INTEGER(0, pages);
246245
OPTIONAL_ARGUMENT_FUNCTION(1, callback);
247246

248-
StepBaton* baton = new StepBaton(backup, callback, pages);
247+
auto* baton = new StepBaton(backup, callback, pages);
249248
backup->GetRetryErrors(baton->retryErrorsSet);
250249
backup->Schedule(Work_BeginStep, baton);
251250
return info.This();
@@ -281,9 +280,9 @@ void Backup::Work_Step(napi_env e, void* data) {
281280

282281
void Backup::Work_AfterStep(napi_env e, napi_status status, void* data) {
283282
std::unique_ptr<StepBaton> baton(static_cast<StepBaton*>(data));
284-
Backup* backup = baton->backup;
283+
auto* backup = baton->backup;
285284

286-
Napi::Env env = backup->Env();
285+
auto env = backup->Env();
287286
Napi::HandleScope scope(env);
288287

289288
if (backup->status == SQLITE_DONE) {
@@ -308,12 +307,12 @@ void Backup::Work_AfterStep(napi_env e, napi_status status, void* data) {
308307
}
309308

310309
Napi::Value Backup::Finish(const Napi::CallbackInfo& info) {
311-
Backup* backup = this;
312-
Napi::Env env = backup->Env();
310+
auto* backup = this;
311+
auto env = backup->Env();
313312

314313
OPTIONAL_ARGUMENT_FUNCTION(0, callback);
315314

316-
Baton* baton = new Baton(backup, callback);
315+
auto* baton = new Baton(backup, callback);
317316
backup->Schedule(Work_BeginFinish, baton);
318317
return info.This();
319318
}
@@ -329,9 +328,9 @@ void Backup::Work_Finish(napi_env e, void* data) {
329328

330329
void Backup::Work_AfterFinish(napi_env e, napi_status status, void* data) {
331330
std::unique_ptr<Baton> baton(static_cast<Baton*>(data));
332-
Backup* backup = baton->backup;
331+
auto* backup = baton->backup;
333332

334-
Napi::Env env = backup->Env();
333+
auto env = backup->Env();
335334
Napi::HandleScope scope(env);
336335

337336
backup->FinishAll();
@@ -369,39 +368,39 @@ void Backup::FinishSqlite() {
369368
}
370369

371370
Napi::Value Backup::IdleGetter(const Napi::CallbackInfo& info) {
372-
Backup* backup = this;
371+
auto* backup = this;
373372
bool idle = backup->inited && !backup->locked && backup->queue.empty();
374373
return Napi::Boolean::New(this->Env(), idle);
375374
}
376375

377376
Napi::Value Backup::CompletedGetter(const Napi::CallbackInfo& info) {
378-
Backup* backup = this;
377+
auto* backup = this;
379378
return Napi::Boolean::New(this->Env(), backup->completed);
380379
}
381380

382381
Napi::Value Backup::FailedGetter(const Napi::CallbackInfo& info) {
383-
Backup* backup = this;
382+
auto* backup = this;
384383
return Napi::Boolean::New(this->Env(), backup->failed);
385384
}
386385

387386
Napi::Value Backup::RemainingGetter(const Napi::CallbackInfo& info) {
388-
Backup* backup = this;
387+
auto* backup = this;
389388
return Napi::Number::New(this->Env(), backup->remaining);
390389
}
391390

392391
Napi::Value Backup::PageCountGetter(const Napi::CallbackInfo& info) {
393-
Backup* backup = this;
392+
auto* backup = this;
394393
return Napi::Number::New(this->Env(), backup->pageCount);
395394
}
396395

397396
Napi::Value Backup::RetryErrorGetter(const Napi::CallbackInfo& info) {
398-
Backup* backup = this;
397+
auto* backup = this;
399398
return backup->retryErrors.Value();
400399
}
401400

402401
void Backup::RetryErrorSetter(const Napi::CallbackInfo& info, const Napi::Value& value) {
403-
Backup* backup = this;
404-
Napi::Env env = backup->Env();
402+
auto* backup = this;
403+
auto env = backup->Env();
405404
if (!value.IsArray()) {
406405
Napi::Error::New(env, "retryErrors must be an array").ThrowAsJavaScriptException();
407406
return;
@@ -413,9 +412,9 @@ void Backup::RetryErrorSetter(const Napi::CallbackInfo& info, const Napi::Value&
413412
void Backup::GetRetryErrors(std::set<int>& retryErrorsSet) {
414413
retryErrorsSet.clear();
415414
Napi::Array array = retryErrors.Value();
416-
int length = array.Length();
417-
for (int i = 0; i < length; i++) {
418-
Napi::Value code = (array).Get(i);
415+
auto length = array.Length();
416+
for (size_t i = 0; i < length; i++) {
417+
Napi::Value code = (array).Get(static_cast<uint32_t>(i));
419418
if (code.IsNumber()) {
420419
retryErrorsSet.insert(code.As<Napi::Number>().Int32Value());
421420
}

Diff for: ‎src/backup.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
121121
Baton(db_, cb_), backup(backup_), filenameIsDest(true) {
122122
backup->Ref();
123123
}
124-
virtual ~InitializeBaton() {
124+
virtual ~InitializeBaton() override {
125125
backup->Unref();
126126
if (!db->IsOpen() && db->IsLocked()) {
127127
// The database handle was closed before the backup could be opened.
@@ -135,6 +135,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
135135
std::set<int> retryErrorsSet;
136136
StepBaton(Backup* backup_, Napi::Function cb_, int pages_) :
137137
Baton(backup_, cb_), pages(pages_) {}
138+
virtual ~StepBaton() override = default;
138139
};
139140

140141
typedef void (*Work_Callback)(Baton* baton);
@@ -169,8 +170,8 @@ class Backup : public Napi::ObjectWrap<Backup> {
169170
retryErrors.Reset();
170171
}
171172

172-
WORK_DEFINITION(Step);
173-
WORK_DEFINITION(Finish);
173+
WORK_DEFINITION(Step)
174+
WORK_DEFINITION(Finish)
174175
Napi::Value IdleGetter(const Napi::CallbackInfo& info);
175176
Napi::Value CompletedGetter(const Napi::CallbackInfo& info);
176177
Napi::Value FailedGetter(const Napi::CallbackInfo& info);
@@ -211,7 +212,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
211212
int remaining;
212213
int pageCount;
213214
bool finished;
214-
std::queue<Call*> queue;
215+
std::queue<std::unique_ptr<Call>> queue;
215216

216217
Napi::Reference<Array> retryErrors;
217218
};

0 commit comments

Comments
 (0)