-1

I am new to coding, so please forgive me if this question seems stupid. I was writing my own List class to get a better understanding of how Lists are structured, but I ran into an issue. I dynamically allocated my list as I added more items to it, and the deconstructor on my program ran just fine with ints. However, as I was testing with std::string, I ran into an issue. It keeps throwing exceptions after my deconstructor is called(, even though (I'm fairly certain) I deleted the memory I allotted alone, and not theirs (read access violation).

I've tried using smart pointers instead of deleting the allocated memory in my deconstuctor, but that ends up having the same issue. Looking online, all I can seem to find is, "only delete with deconstructors," and, "don't have exception handling in deconstructors." Both of which are not even issues with what I've written.

Here is firstly, the relevant code (in my mind) to solving this.

#include <string>
#include <iostream>
using std::cout;
using std::cin;
using std::string;

template <class type>
class List
{
    struct Node
    {
        type data;
        Node* next;
    };

public:

    List();

    ~List();

    void addToList(type var);
private:

    Node head;
    Node *last, *lastAcc;

    unsigned int length, prevPos;
};

template <class type>
List<type>::~List()
{
    Node *prevPtr;
    lastAcc = head.next;

    while (lastAcc->next) // While the next pointer leads to something
    {
        // Go to that something, and delete the item you were on

        prevPtr = lastAcc;
        lastAcc = lastAcc->next;
        delete prevPtr;
    }

    delete lastAcc;
}


template <class type>
void List<type>::addToList(type var)
{
    if (length)
    {
        last->next = new Node;
        last = last->next;
        last->data = var;
    }
    else
    {
        head.data = var;
    }

    lastAcc = last;
    prevPos = length++;
}

template <class type>
List<type>::List()
{
    head.next = 0;
    prevPos = 0;
    lastAcc = last = &head;
    length = 0;
}

int main()
{
    string boi[] = { "Today is a good day", "I had an apple", "It tasted delicious" };

    List<string> multiString;

    for (int i = 0; i < 3; i++)
    {
        multiString.addToList(boi[i]);
    }
    return 0;
}

I expected the code to run just fine, and if I made an error, I thought the error would show up on my code. Not on std::string. Any help would be greatly appreciated.

[Edit] On an added note, [lastAcc] is abbreviated for last accessed; it's just something I implemented to make going through the lists faster than just having to start from 0 every time. [prevPos] shows the position of [lastAcc] in the list. Let me know if you need to see more of my code or explain anything~!

Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
Michael D.
  • 35
  • 5
  • 2
    `#include "List.cpp"` Don't do that. – πάντα ῥεῖ Mar 29 '19 at 07:25
  • 1
    Welcome to Stack Overflow! Being new here, you should take the [tour] and read [ask]. Concerning your problem, you need to extract a [mcve] first. Chances are you will find the error yourself doing that, which is exactly why it is required. – Ulrich Eckhardt Mar 29 '19 at 07:25
  • The reason I used #include "List.cpp" is because I was getting a linking error with my template. An online guide pointed out that it was necessary to include the .cpp file instead of the .h file in order to get the templates to extend. – Michael D. Mar 29 '19 at 07:33
  • It is very hard to say what is causing the error. I doubt it comes from `std::string` anyway. On the other hand If you need specific list implementation, why don't you just wrap `std::vector` for your needs ? – Diodacus Mar 29 '19 at 07:33
  • Can you share the source of `decNumLists` you're calling at the end of the destructor? – Mureinik Mar 29 '19 at 07:34
  • Its a static function. Here's what it does. ``` template void List::decNumLists() { numLists--; } ``` – Michael D. Mar 29 '19 at 07:35
  • @MichaelD. Templates are typically placed in header files (unless you go with C++14 or later - it is then possible to place implementation in .cpp file). The tutorial you took mus have been very old and enforced bad behaviors. – Diodacus Mar 29 '19 at 07:36
  • I'd rather see `addToList` function. For example `while (lastAcc->next)` will fail if you don't initialize `next` to `nullptr`, right? – freakish Mar 29 '19 at 07:38
  • As for initializing next to nullptr, I set it to zero in the first and the second constructors. is that equivalent? – Michael D. Mar 29 '19 at 07:39
  • @freakish I see what you mean though about the last part, if head.next is 0 when called, delete lastAcc->next would probably give me an error. I'll fix that now, thank you – Michael D. Mar 29 '19 at 07:46
  • @MichaelD. That's for sure. Also I recommend running the code through a debugger. It's a bit long. – freakish Mar 29 '19 at 07:48

1 Answers1

1

you aren't initialising last->next in addToList so iteration in your destructor falls off the end of the list. The correct code is:

void List<type>::addToList(type var)
{
    if (length)
    {
        last->next = new Node();
        last = last->next;
        last->data = var;
    }
    else
    {
        head.data = var;
    }

    lastAcc = last;
    prevPos = length++;
}

The difference is new Node() rather than new Node. The first value initialises POD types, the second doesn't.

Alternatively if you define a constructor for Node then new Node and new Node() will be equivalent:

struct Node
{
    Node(): next( 0 ) {}
    type data;
    Node* next;
};

For a small efficiency gain you could move your value into your node to prevent copies:

struct Node
{
    Node(): next( 0 ) {}
    Node(type && data): data( std::move( data ) ), next( 0 ) {}
    type data;
    Node* next;
};

template <typename T>
void addToList(T&& var)
{
    if (length)
    {
        last->next = new Node(std::move(var));
        last = last->next;
    }
    else
    {
        head.data = std::move(var);
    }

    lastAcc = last;
    prevPos = length++;
}
Alan Birtles
  • 32,622
  • 4
  • 31
  • 60