ivanseidel / LinkedList

🔗 A fully implemented LinkedList made to work with general Microcontrollers and Arduino projects
MIT License
347 stars 120 forks source link

2 bugs in the remove() #4

Closed iisfaq closed 10 years ago

iisfaq commented 10 years ago

There are 2 bugs in your remove method

template<typename T>
T LinkedList<T>::remove(int index){
    if(index < 0 || index >= _size)
        return T();

    if(index == 0)
        return shift();

    if(index - 1 == _size)
        return pop();

    ListNode<T> *tmp = getNode(index - 1);
    ListNode<T> *toDelete = tmp->next;
    tmp->next = tmp->next->next;
    delete(toDelete);
    _size--;
    isCached = false;

    return T();
}

Bug 1

    if(index - 1 == _size)
        return pop();

This can never work!

For example you have 5 items in a list, and you want to remove the last item.

index =4 for the 5th element in the list.

Your code will never work 4-1 = 3, and comparing 3 to 5 don't work

So you need to change that to

    if(index == _size-1)
        return pop();

Bug 2

At the end of the method you have this statement

return T();

This creates a new instance of T instead of returning the current T for the node you are deleting. Here is my corrected code.

        ListNode<T> *tmp = getNode(index - 1);
    ListNode<T> *toDelete = tmp->next;
-->        T ret = toDelete->data;
    tmp->next = tmp->next->next;
    delete(toDelete);
    _size--;
    isCached = false;
--> return ret;

The new corrected code

template<typename T>
T LinkedList<T>::remove(int index){
    if (index < 0 || index >= _size)
    {
        return T();
    }

    if(index == 0)
        return shift();

    if (index == _size-1)
    {
        return pop();
    }

    ListNode<T> *tmp = getNode(index - 1);
    ListNode<T> *toDelete = tmp->next;
    T ret = toDelete->data;
    tmp->next = tmp->next->next;
    delete(toDelete);
    _size--;
    isCached = false;
    return ret;
    //return T();
}

Chris

ivanseidel commented 10 years ago

Great! I guess I implemented those methods, but actually never used...

Do you want to commit it, or want me to doit? You can doit directly in GitHub if you want @iisfaq =)

iisfaq commented 10 years ago

I do not know how to. I would like to but not sure what to do.

ivanseidel commented 10 years ago

You can even do it from the GitHub, without any command line...

If you want to, you can learn it here:

Or, tell me and I doit from here...