switch to unique_ptr in API class && finished API::Scan()

kkli08 · Sep 2, 2024 · cb743bd · cb743bd
1 parent 0ff9401
commit cb743bd
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 81 deletions.
diff --git a/api/api.cpp b/api/api.cpp
@@ -16,6 +16,18 @@ namespace kvdb {
   void API::Open(string db_name){
     std::cout << "Opening database " << db_name << std::endl;
 
+    if (is_open) {
+      throw runtime_error("Database is already open.");
+    }
+
+    // Allocate or reallocate memtable and index
+    if (!memtable) {
+      memtable = make_unique<Memtable>();
+    }
+    if (!index) {
+      index = make_unique<SSTIndex>();
+    }
+
     // c++17 new feature
     // Define the path to the database directory
     fs::path db_path = db_name;
@@ -60,7 +72,6 @@ namespace kvdb {
     is_open = false;
     // clean up memory
     delete info;
-    cleanup();
   }
 
   /*
@@ -121,15 +132,6 @@ namespace kvdb {
     return -1;
   }
 
-  // memory cleanup function
-  void API::cleanup() {
-    // Check if memtable is not null
-    if (memtable) {
-      delete memtable;  // Delete the dynamically allocated memtable
-      memtable = nullptr;  // Set pointer to nullptr to avoid dangling pointer
-    }
-    std::cout << "Cleanup completed." << std::endl;
-  }
 
   /*
    * unordered_map<LL, LL> API::Scan(LL, LL)
@@ -154,14 +156,6 @@ namespace kvdb {
     return result;
   }
 
-  API::~API() {
-    // Check if memtable is not null
-    if (memtable) {
-      delete memtable;  // Delete the dynamically allocated memtable
-      memtable = nullptr;  // Set pointer to nullptr to avoid dangling pointer
-    }
-  }
-
   // helper function
   void API::set_path(fs::path _path) {
     path = _path;

diff --git a/api/api.h b/api/api.h
@@ -9,32 +9,38 @@
 #include <filesystem> // C++17 lib
 #include <unordered_map>
 #include "SSTIndex.h"
+#include <memory>
 
 namespace fs = std::filesystem;
 using namespace std;
 namespace kvdb {
     class API {
     public:
-        API() : memtable_size(1e4), memtable(new Memtable()), index(new SSTIndex()) {};
-        ~API();
+        API()
+                    : memtable_size(1e4),
+                      memtable(make_unique<Memtable>()),
+                      index(make_unique<SSTIndex>())
+        {};
+
+        ~API() = default;
         void Open(string db_name);
         void Close();
         void Put(long long key, long long value);
         long long Get(long long key);
         unordered_map<long long, long long> Scan(long long small_key, long long large_key);
-        Memtable* GetMemtable() const {return memtable;};
+        Memtable* GetMemtable() const {return memtable.get();};
         void IndexCheck();
 
     private:
-        Memtable *memtable;
-        SSTIndex *index;
+        unique_ptr<Memtable> memtable;
+        unique_ptr<SSTIndex> index;
+
         int memtable_size;
         fs::path path; // path for store SSTs
-        bool is_open;
+        bool is_open = false;
         // helper function: set memtable_size
         void set_memtable_size(int _size);
         void set_path(fs::path);
-        void cleanup();
         void check_if_open() const {
             if (!is_open) {
                 throw runtime_error("Database is not open. Please open the database before performing operations.");

diff --git a/tests/api_unittest.cpp b/tests/api_unittest.cpp
@@ -58,30 +58,41 @@ TEST(APITest, OpenExistingDatabase) {
     fs::remove_all(db_name);  // Clean up the created directory
 }
 
-TEST(APITest, CloseAndCleanup) {
-    API* api = new API();
-    std::string db_name = "test_db_cleanup";
 
-    // Open a database to initialize the API
-    api->Open(db_name);
+// replace destructor with smart pointer
 
-    testing::internal::CaptureStdout();
-    api->Close();
-    std::string output = testing::internal::GetCapturedStdout();
-
-    // Check that the cleanup message is printed
-    EXPECT_TRUE(output.find("Cleanup completed.") != std::string::npos);
-
-    // Verify that memtable is cleaned up (set to nullptr)
-    EXPECT_EQ(api->GetMemtable(), nullptr);  // Assuming GetMemtable() is a getter for the memtable pointer
+// TEST(APITest, CloseAndCleanup) {
+//     API* api = new API();
+//     std::string db_name = "test_db_cleanup";
+//
+//     // Open a database to initialize the API
+//     api->Open(db_name);
+//
+//     testing::internal::CaptureStdout();
+//     api->Close();
+//     std::string output = testing::internal::GetCapturedStdout();
+//
+//     // Check that the cleanup message is printed
+//     EXPECT_TRUE(output.find("Cleanup completed.") != std::string::npos);
+//
+//     // Verify that memtable is cleaned up (set to nullptr)
+//     EXPECT_EQ(api->GetMemtable(), nullptr);  // Assuming GetMemtable() is a getter for the memtable pointer
+//
+//     // Clean up
+//     delete api;
+//     fs::remove_all(db_name);  // Clean up the created directory
+// }
 
-    // Clean up
-    delete api;
-    fs::remove_all(db_name);  // Clean up the created directory
+TEST(APITest, reOpen) {
+    API* api = new API();
+    std::string db_name = "test_db_reopen";
+    api->Open(db_name);
+    api->Close();
+    // api = new API();
+    api->Open(db_name);
+    api->Close();
 }
 
-
-
 // 10 k pairs Put & Get
 TEST(APITest, InsertAndRetrieve10KKeyValuePairs) {
     API* api = new API();
@@ -221,40 +232,40 @@ TEST(APITest, ScanAcrossSSTsAndMemtable) {
     fs::remove_all(db_name);  // Remove the test database directory
 }
 
-// TEST(APITest, ScanWithOverlappingSSTAndMemtableData) {
-//     API* api = new API();
-//     std::string db_name = "test_db";
-//
-//     // Open the database
-//     api->Open(db_name);
-//
-//     // Insert 1e5 key-value pairs using the API's Put method
-//     for (long long i = 1; i <= 1e5; ++i) {
-//         api->Put(i, i * 10);
-//     }
-//
-//     // Flush memtable to SST (simulate this by closing and reopening the database)
-//     api->Close();
-//     api->Open(db_name);
-//
-//     // Insert another set of data into the memtable
-//     for (long long i = 1e5 + 1; i <= 1e5 + 5000; ++i) {
-//         api->Put(i, i * 10);
-//     }
-//
-//     // Perform a scan that overlaps SST files and the memtable
-//     long long small_key = 99000;
-//     long long large_key = 105000;
-//     unordered_map<long long, long long> result = api->Scan(small_key, large_key);
-//
-//     // Validate the results
-//     EXPECT_EQ(result.size(), large_key - small_key + 1);
-//     for (long long i = small_key; i <= large_key; ++i) {
-//         EXPECT_EQ(result[i], i * 10);
-//     }
-//
-//     // Cleanup
-//     api->Close();
-//     delete api;
-//     fs::remove_all(db_name);  // Remove the test database directory
-// }
+TEST(APITest, ScanWithOverlappingSSTAndMemtableData) {
+    API* api = new API();
+    std::string db_name = "test_db";
+
+    // Open the database
+    api->Open(db_name);
+
+    // Insert 1e5 key-value pairs using the API's Put method
+    for (long long i = 1; i <= 1e6; ++i) {
+        api->Put(i, i * 10);
+    }
+
+    // Flush memtable to SST (simulate this by closing and reopening the database)
+    api->Close();
+    api->Open(db_name);
+
+    // Insert another set of data into the memtable
+    for (long long i = 1e5 + 1; i <= 1e5 + 5000; ++i) {
+        api->Put(i, i * 10);
+    }
+
+    // Perform a scan that overlaps SST files and the memtable
+    long long small_key = 99000;
+    long long large_key = 105000;
+    unordered_map<long long, long long> result = api->Scan(small_key, large_key);
+
+    // Validate the results
+    EXPECT_EQ(result.size(), large_key - small_key + 1);
+    for (long long i = small_key; i <= large_key; ++i) {
+        EXPECT_EQ(result[i], i * 10);
+    }
+
+    // Cleanup
+    api->Close();
+    delete api;
+    fs::remove_all(db_name);  // Remove the test database directory
+}