Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

师兄,发现几个问题 #1

Open
xuhao531799712 opened this issue Jan 3, 2021 · 1 comment
Open

师兄,发现几个问题 #1

xuhao531799712 opened this issue Jan 3, 2021 · 1 comment

Comments

@xuhao531799712
Copy link

  1. skiplist.h 中的 get_random_level() 函数中的 n 的初始值应该是 0 不应该是 1。
template<typename K, typename V>
int SkipList<K, V>::get_random_level(){

    int k = 1;   // 这个地方应该是 0 
    while (rand() % 2) {
        k++;
    }
    k = (k < _max_level) ? k : _max_level;
    return k;
};

如果 k 的初始值是1的话,那么插入的节点的 level 都是大于等于1的,那么在整个跳表中,第0层的索引和第1层的索引就完全一样没有区分度了。
2. skiplist.h 中的 delete_element() 中,只是把节点从指针链表中删除了,但实际的节点没有删除,会造成内存泄漏

    delete current;    // 应该有删除本节点的语句
    _element_count --;
  1. skiplist.h 中的 ~SkipList() 中,也只删除了一下头节点,本跳表中的其他节点也都没有进行删除
template<typename K, typename V> 
SkipList<K, V>::~SkipList() {

    if (_file_writer.is_open()) {
        _file_writer.close();
    }
    if (_file_reader.is_open()) {
        _file_reader.close();
    }

    // 此处为所添加的部分 循环删除所有节点
    Node<K, V>* current = _header->forward[0];
    while(current) {
        Node<K, V>* tmp = current->forward[0];
        delete current;
        current = tmp;
    }

    delete _header;
}
  1. 另外有一处小优化的地方,在 delete_element() 里,进行索引数组更新的地方
        // start for lowest level and delete the current node of each level
        for (int i = 0; i <= _skip_list_level; i++) {

            // if at level i, next node is not target node, break the loop.
            if (update[i]->forward[i] != current) 
                break;

            update[i]->forward[i] = current->forward[i];
        }

可以改为

    for (int i = current->node_level; i >= 0; --i) {
        update[i]->forward[i] = current->forward[i];
    }

因为从被删除节点的 node_level 可以得到它的层级,那么在 i<=node_level 的层级上,一定满足原来代码中的 update[i]->forward[i] == current 的条件。

师兄,这些是我个人见解,有不对的地方帮我指正出来吧

@youngyangyang04
Copy link
Owner

  1. skiplist.h 中的 get_random_level() 函数中的 n 的初始值应该是 0 不应该是 1。
template<typename K, typename V>
int SkipList<K, V>::get_random_level(){

    int k = 1;   // 这个地方应该是 0 
    while (rand() % 2) {
        k++;
    }
    k = (k < _max_level) ? k : _max_level;
    return k;
};

如果 k 的初始值是1的话,那么插入的节点的 level 都是大于等于1的,那么在整个跳表中,第0层的索引和第1层的索引就完全一样没有区分度了。
2. skiplist.h 中的 delete_element() 中,只是把节点从指针链表中删除了,但实际的节点没有删除,会造成内存泄漏

    delete current;    // 应该有删除本节点的语句
    _element_count --;
  1. skiplist.h 中的 ~SkipList() 中,也只删除了一下头节点,本跳表中的其他节点也都没有进行删除
template<typename K, typename V> 
SkipList<K, V>::~SkipList() {

    if (_file_writer.is_open()) {
        _file_writer.close();
    }
    if (_file_reader.is_open()) {
        _file_reader.close();
    }

    // 此处为所添加的部分 循环删除所有节点
    Node<K, V>* current = _header->forward[0];
    while(current) {
        Node<K, V>* tmp = current->forward[0];
        delete current;
        current = tmp;
    }

    delete _header;
}
  1. 另外有一处小优化的地方,在 delete_element() 里,进行索引数组更新的地方
        // start for lowest level and delete the current node of each level
        for (int i = 0; i <= _skip_list_level; i++) {

            // if at level i, next node is not target node, break the loop.
            if (update[i]->forward[i] != current) 
                break;

            update[i]->forward[i] = current->forward[i];
        }

可以改为

    for (int i = current->node_level; i >= 0; --i) {
        update[i]->forward[i] = current->forward[i];
    }

因为从被删除节点的 node_level 可以得到它的层级,那么在 i<=node_level 的层级上,一定满足原来代码中的 update[i]->forward[i] == current 的条件。

师兄,这些是我个人见解,有不对的地方帮我指正出来吧

很细心呀,可以可以👍,我抽空看一下哈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants