-3

So i have a Linked list implementation of my own and it can successfully keep integers and call them when needed with overloaded [] operator but when it comes to storing a class in my linked list, it seems that i can't call the class appropriately (using the same [] operator). Called functions and members of my Linked List;

#include <iostream>
#include <assert.h>

template<typename T>
struct node {
    T data;
    node<T>* next;
};

template<typename T>
class Vectem {
private:
    node<T>* head;
    node<T>* last;
    int lenght;
public:
    void insert(T value) {
        last->next = new node<T>;
        last = last->next;
        last->data = value;
        last->next = NULL;
        if (isEmpty()) {
            head = last;
        }
        lenght++;
    }
    node<T>* search(int indx) {
        node<T>* current;
        current = head;
        int count=0;
        while (current != NULL) {
            if (count == indx) {
                break;
            }
            current = current->next;
            count++;
        }
        return current;
    }
    T& operator [](int indx) {
        assert(indx >= lenght - 1);
        T result;
        result = search(indx)->data;
        return result;
    }
};

And here is the main function and the class that i try to store;

#include <iostream>
#include <fstream>
#include <string>
#include "VectemLibrary.h"
class word {
public:
    std::string value;
    int count;
    word(std::string value, int count): value(value),count(count) {

    }
    word() {
        value = "NOT ASSIGNED";
        count = 0;
    }
    word(const word& w1) {
        value = w1.value;
        count = w1.count;
    }
    ~word() {
        std::cout << "Word Destroyed" << std::endl;
    }
};
int main()
{
    Vectem<word> wordContainer;
    word newWord("hello", 1);
    wordContainer.insert(newWord);
    std::cout << wordContainer[0].value;
    
}

Visual studio gave me the expection with this message at the last line where i call the first member of linked list with []; Exception thrown at 0x7A0CF3BE (ucrtbased.dll) in Top 10 words.exe: 0xC0000005: Access violation reading location 0xCCCCCCCC. I think that my lack of experience with pointers may have caused the problem but if you see something that i can't, Please enlighten me.

GEM
  • 1
  • 3
  • 4
    `T result; ...return result;` You are returning a reference to the local variable. Try just `return search(indx)->data;` – 001 Jan 15 '21 at 00:14
  • Also, in `insert()` you have `last->next = new node;`. But what is the value of `last` the very first time you call this function? – 001 Jan 15 '21 at 00:24
  • [Why should I always enable compiler warnings?](https://stackoverflow.com/questions/57842756/why-should-i-always-enable-compiler-warnings) – JaMiT Jan 15 '21 at 00:51

2 Answers2

1

There are other problems with the code you posted as well (e.g. isEmpty() is not declared or defined), but I'll focus on the issue you explicitly mentioned.

In your operator:

T& operator [](int indx) {
    assert(indx >= lenght - 1);
    
    // You declare this variable on the stack
    T result;
    
    result = search(indx)->data;
    
    // And then you return this variable by reference; this is not okay
    return result;
}

As mentioned in my code comments (and by @Johnny Mopp in his comment to your post), you shouldn't (can't) return a reference or pointer to a variable declared within the returning function and constructed on the stack. Anything on the stack will be destroyed once the function call ends, so any returned pointers or references to such variables will be dangling references; using said pointers or references will result in undefined behavior.

So you don't want to return a reference to a stack-allocated variable like result; you want to return a reference to the data within the node itself (which is allocated on the heap by insert()), as it will still be a valid reference after the function returns:

return search(indx)->data;

Alexander Guyer
  • 2,063
  • 1
  • 14
  • 20
  • All other funcions are implemented. I just didn't share the whole code to not waste your time reading it because they work just fine and returning search(indx)->data just worked perfectly. Thank you and @Johnny Mopp. – GEM Jan 15 '21 at 10:56
1

There are several problems with your code, but the most important is that you are not initializing the head, last, or lenght members of Vectem at all. An Access Violation error at address 0xCCCCCCCC is a good indication that uninitialized memory is being accessed, as some compilers/setups fill uninitialized memory with 0xCC bytes, thus head and last are initially 0xCCCCCCCC in your case.

You need to add appropriate constructors to Vectem (as well as a destructor, a copy constructor, and a copy assignment operator, per the Rule of 3), eg:

template<typename T>
class Vectem {
private:
    node<T>* head;
    node<T>* last;
    int lenght;
public:
    Vectem() : head(NULL), last(NULL), lenght(0) {}

    Vectem(const Vectem &src) : head(NULL), last(NULL), lenght(0)
    {
        // copy src's data to *this as needed ...
    }

    ~Vectem()
    {
        // cleanup *this as needed ...
    }

    Vectem& operator=(const Vectem &rhs)
    {
        if (&rhs != this) {
            // clear *this, and copy rhs's data to *this, as needed ...
        }
        return *this;
    }

    ...
};

Or, in C++11 and later, you can initialize the members directly in their declarations (also, be sure to add a move constructor and a move assignment operator, per the Rule of 5), eg:

template<typename T>
class Vectem {
private:
    node<T>* head = nullptr;
    node<T>* last = nullptr;
    int lenght = 0;
public:
    Vectem() = default;

    Vectem(const Vectem &src)
    {
        // copy src's data to *this as needed ...
    }

    Vectem(Vectem &&src) : head(src.head), last(src.last), lenght(src.lenght) 
    {
        src.head = nullptr;
        src.last = nullptr;
        src.lenght = 0;
    }

    ~Vectem()
    {
        // cleanup *this as needed ...
    }

    Vectem& operator=(const Vectem &rhs)
    {
        if (&rhs != this) {
            // clear *this, and copy rhs's data to *this, as needed ...
        }
        return *this;
    }

    Vectem& operator=(Vectem &&rhs)
    {
        // clear *this as needed...

        head = rhs.head; rhs.head = nullptr;
        last = rhs.last; rhs.last = nullptr;
        lenght = rhs.lenght; rhs.lenght = 0;

       return *this;
    }

    ...
};

That being said, insert() is also buggy, as it is dereferencing last before checking that last is actually pointing at a valid node. Try something more like this instead:

void insert(T value) {
    node<T> *n = new node<T>{value, NULL};
    if (!head) head = n;
    if (last) last->next = n;
    last = n;
    ++lenght;
}

Alternatively:

void insert(T value) {
    node<T> **p = (last) ? &(last->next) : &head;
    *p = new node<T>{value, NULL};
    last = *p;
    ++lenght;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Actually i didn't post the whole library to not waste your time reading all of it. I tought if i just show the fuctions that are being used it would be better for everyone but i suppose i was wrong. Vectem has all needed constructors and works fine but you are right i should've posted them as well. Thank you for your efforts. It has been useful for me just to look at your implementation. – GEM Jan 15 '21 at 10:47