C++: What is best way to destruct this code?

Question

I want to know proper way of destructing this code as I'm facing many random issues. However, this piece of code is working as I'm calling ~CacheFrame() first and than assinging nullptr to pageMap inside ~LruCache()

#include <iostream>
#include <unordered_map>
#include <list>

using namespace std;

struct Node {
    int pageNumber;
    Node *next;
    Node *prev;
    Node(int pageNumber, Node *prev, Node *next) {
        this->pageNumber = pageNumber;
        this->next = next;
        this->prev = prev;
    }
};

class CacheFrame {
    const size_t maxSize;
    int size;
    Node *head, *tail;
public:
    CacheFrame(int maxSize) : maxSize(maxSize) {
        size = 0;
        head = tail = nullptr;
    }

    ~CacheFrame() {
        cout << "CacheFrame Destructor called" << endl;
        tail = nullptr;
        while (head) {
            Node *temp = head->next;
            head->next = nullptr;
            head->prev = nullptr;
            delete head;
            head = temp;
        }
    }

    void MovePageToFront(Node* page) {
        if (page == nullptr || page == head) {
            return;
        }
        if (page == tail) {// point tail to second last element
            tail = tail->prev;
            tail->next = nullptr;
        }
        page->prev->next = page->next;
        if (page->next)
            page->next->prev = page->prev;
        page->next = head;
        head->prev = page;
        head = page;
    }

    Node* InsertPage(int pageNumber) {
        Node *page = new Node(pageNumber, nullptr, nullptr);
        if (!head) {
            head = tail = page;
            size = 1;
            return head;
        }
        if (size < maxSize) {
            size++;
        } else {
            Node *temp = tail;
            tail = tail->prev;
            tail->next = nullptr;
            delete temp;
        }
        page->next = head;
        head->prev = page;
        head = page;
        return page;
    }

    size_t getCurrentSize() {
        return size;
    }

    size_t getMaxSize() {
        return maxSize;
    }

    int getLastPageNumber() {
        return tail->pageNumber;
    }

    void printFrame() {
        Node *start = head;
        cout << "Frame" << endl;
        while (start) {
            cout << "-----" << endl;
            cout << "| " << start->pageNumber << " |" << endl;
            start = start->next;
        }
        cout << "-----"<< endl;
    }

};

class LruCache {
    const size_t size;
    CacheFrame frame;
    unordered_map<int, Node*> pageMap;
public:
    LruCache(int size): size(size), frame(CacheFrame(size)) {}
    ~LruCache() {
        cout << "LruCache Destructor called" << endl;
        frame.~CacheFrame();
        for (auto it: pageMap) {
            cout << "Deleting " << it.first << " from map" << endl;
            it.second = nullptr;
            pageMap.erase(it.first);
//            if (it.second) {
//                it.second->next = nullptr;
//                it.second->prev = nullptr;
//                delete it.second;
//            }
        }
    }
    void accessPage(int pageNumber) {
        cout << "Access Page : " << pageNumber << endl;
        if (pageMap.find(pageNumber) != pageMap.end()) {// page hit
            cout << "Page Hit" << endl;
            frame.MovePageToFront(pageMap[pageNumber]);
        } else {//page miss
            cout << "Page Miss" << endl;
            if (frame.getMaxSize() == frame.getCurrentSize()) {
                pageMap.erase(frame.getLastPageNumber());
            }
            pageMap[pageNumber] = frame.InsertPage(pageNumber);
        }
        frame.printFrame();
    }
};

int main() {
    LruCache cache = LruCache(3);
    int array[] = {4,2,1,1,4,3,7,8,3};
    for (auto it: array) {
        cache.accessPage(it);
    }
    return 0;
}

Please post a smaller example. Or consider codereview.stackexchange.com (but again, with a smaller example) — Mateen Ulhaq, Aug 17 '17 at 22:30
Looks like frame dtor gets executed twice, once explicitly called once implicitly for destroying Lrucache. — Yunnosch, Aug 17 '17 at 22:31
Lol i mean this line in your source `// Copyright © 2017 Rohit Pal. All rights reserved.` — kocica, Aug 17 '17 at 22:34
I didnt mean to delete it, i just wondered why are you using it :-) — kocica, Aug 17 '17 at 22:37
But seriously, this should be well covered in your text. If not, here is a list of good texts: https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list — user4581301, Aug 17 '17 at 22:53

score 0 · Accepted Answer · answered Aug 17 '17 at 22:42

0

C++: What is best way to destruct this code?

You mean to free resources.

In case of unordered_map

The way to do it right is not to do it. A unordered_map will automatically release resources when it's destroyed for anything allocated automatically.

Unless you allocated the values with new, you don't delete them.

In case of linked list

~CacheFrame()
{
    Node *temp = head->next, *temp2;

    while(temp != NULL)
    {
        temp2 = temp;
        temp = temp->next;
        delete temp2;
    }
}

answered Aug 17 '17 at 22:42

kocica

6,412
2
14
35

I think you are right about this _The way to do it right is not to do it. A unordered_map will automatically release resources when it's destroyed for anything allocated automatically._ – Rohit Pal Aug 17 '17 at 22:52
Yes these standard containers are designed to be easy to use. – kocica Aug 17 '17 at 22:54

C++: What is best way to destruct this code?

1 Answers1