-1

I wrote this code while studying linked lists on leetcode. In this problem, at the end, where deleteAtIndex function is called with index 6, the screen goes blank. It is working for all other values where the index is valid. But for invalid values of the index, there is no output.

/*Delete the index-th node in the linked list, if the index is valid.
Below is the function I wrote.*/

void deleteAtIndex(int index) {

    Node* current =start;
    int x=0;
    int i;

    while(current != NULL){
        x++;
        current = current->next;
    }

    if(index == 0){
            current =start;
        start = current->next;
    }
    else if(index >0 && index <=x ){
        current =start;
        for(i=0; i<index-1; i++){
            current = current ->next;
        }
        current->next= current->next->next;
    }


    if (index > x || index < 0) {
        cout << "Invalid Index" << endl;
    }
    return;
}
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • 1
    In the C++ Standard Library there are numerous list-like containers, so you don't need to implement those yourself. – Ron Aug 14 '19 at 11:57
  • 1
    Try debuggind the code – Sebastian D'Agostino Aug 14 '19 at 12:00
  • 2
    Show a [mcve] including minimal input (if any) as well as actual and expected output The problem might be elsewhere than in the `deleteAtIndex` function. – Jabberwocky Aug 14 '19 at 12:01
  • 2
    There is a problem when you try to delete the _last_ node of the list. Take a piece of paper and a pencil and draw a linked list containing for example 2 elements. You should be able to figure out why deleting the last element of the list won't work. – Jabberwocky Aug 14 '19 at 12:26

1 Answers1

2

For starters the function does not make sense because it deletes nothing. In fact it tries to exclude a node from the list.

However the function has undefined behavior.

Let's assume that start is equal to NULL and index is also equal to 0. In this case due to these statements

if(index == 0){
        current =start;
    start = current->next;
            ^^^^^^^^^^^^^
}

there is an access to the memory using a null-pointer.

Now let's assume that the start is not equal to NULL and it is the only node in the list. And index is equal to 1.

In this case x will be equal to 1. And due to these statements

else if(index >0 && index <=x ){
    current =start;
    for(i=0; i<index-1; i++){
        current = current ->next;
    }
    current->next= current->next->next;
                   ^^^^^^^^^^^^^^^^^^^^
}

again there is an access to memory using a null-pointer.

So the function is entirely wrong.

Pay attention to that the function parameter should have an unsigned integer type. The most appropriate type is size_t.

And if there is a function that uses a method by accessing a node by index then the list should keep the number of nodes in it.

If the program is indeed written as a C++ program then the function can look the following way as it is shown in the simplified demonstrative program below. Investigate it.

#include <iostream>

class List
{
public:
    explicit List() = default;

    List( const List & ) = delete;
    List & operator =( const List & ) = delete;

    void push_front( int value )
    {
        head = new Node { value, head };
    }

    std::ostream & out( std::ostream &os = std::cout ) const
    {
        for ( Node *current = head; current != nullptr; current = current->next )
        {
            os << current->value << " -> ";
        }
        os << "nullptr";

        return os;
    }

    bool deleteAtIndex( size_t n )
    {
        Node **current = &head;

        while ( *current != nullptr && n )
        {
            current = &( *current )->next;
            --n;
        }

        bool success = *current != nullptr;

        if ( success )
        {
            Node *tmp = *current;
            *current = ( *current )->next;
            delete tmp;
        }

        return success;
    }

protected:
    struct Node
    {
        int value;
        Node *next;
    } *head = nullptr;
};

std::ostream & operator <<( std::ostream &os, const List &lst )
{
    return lst.out( os );
}

int main() 
{
    const size_t N = 10;

    List lst;

    for ( size_t i = N; i != 0; --i ) lst.push_front( int( i - 1 ) );

    std::cout << lst << '\n';

    std::cout << '\n';

    for ( size_t i = N; i != 0; --i ) 
    {
        lst.deleteAtIndex( i - 1 );
        std::cout << lst << '\n';
    }

    return 0;
}

The program output is

0 -> 1 -> 2 -> 3 -> 4 -> 5 -> 6 -> 7 -> 8 -> 9 -> nullptr

0 -> 1 -> 2 -> 3 -> 4 -> 5 -> 6 -> 7 -> 8 -> nullptr
0 -> 1 -> 2 -> 3 -> 4 -> 5 -> 6 -> 7 -> nullptr
0 -> 1 -> 2 -> 3 -> 4 -> 5 -> 6 -> nullptr
0 -> 1 -> 2 -> 3 -> 4 -> 5 -> nullptr
0 -> 1 -> 2 -> 3 -> 4 -> nullptr
0 -> 1 -> 2 -> 3 -> nullptr
0 -> 1 -> 2 -> nullptr
0 -> 1 -> nullptr
0 -> nullptr
nullptr
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335