Thread: new and my Linked_list class

I have a question regarding the use of new within my class definition: when I call delete in main, does it free all memory allocated for each node and for list or just list?

Code:

//linked_list.h
 
#ifndef LINKED_LIST_H

#define LINKED_LIST_H

#include <iostream>
#include <string>

class Linked_list {
public:
	Linked_list();
	Linked_list *startnode(std::string data, Linked_list *node);
	Linked_list *addnode(std::string data, Linked_list *node);
	Linked_list *delnode(Linked_list *node);
	void display(Linked_list *a_list) const;
private:
	Linked_list *link;
	std::string data;
};

#endif

Code:

// Linked_list.cpp
 
#include "Linked_list.h"

using namespace std;

Linked_list::Linked_list() : link(0), data(" ") {}

Linked_list *Linked_list::startnode(std::string data, Linked_list *node)
{ 
	node->data = data; 
	node->link = 0; 
	return node;
}

Linked_list *Linked_list::addnode(std::string data, Linked_list *node)
{ 
	Linked_list *next = new Linked_list; 
	next->data = data; 
	next->link = node; 
	return next; 
}

Linked_list *Linked_list::delnode(Linked_list *node)
{
	Linked_list *head = node;
	head = head->link;
	node->link = 0;
	return head;
}

void Linked_list::display(Linked_list *a_list) const
{
	while(a_list != 0)
	{
		cout << a_list->data << endl;
		a_list = a_list->link;
	}
}

Code:

#include "Linked_list.h"

using namespace std;

int main(int argc, char *argv[])
{	
	Linked_list *a_list = new Linked_list;
	string word = "Jesus";
	
	a_list = a_list->startnode(word, a_list);
	
	word = "Glory to God!";
	
	a_list = a_list->addnode(word, a_list);
		
	a_list->display(a_list);
	
	delete a_list;
	
	return 0;
}

Thank you in advance for your kind support of my question.

Is there a reason you are using new in main instead of just creating an instance of your list?

Code:

Linked_list a_list;

Jim

Originally Posted by jimblumberg

Is there a reason you are using new in main instead of just creating an instance of your list?

Code:

Linked_list a_list;

Jim

Actually Linked_list *a_list; I used new in main() because I was receiving EXEC_BAD_ACCESS errors indicating I did not have any memory set aside for my pointer to type Linked_list.

Originally Posted by rodrigorules

Imanuel, you're right in thinking that it does not free the memory from each of your nodes.

If you want to make sure this happens on 'delete' or when your list goes out of scope, look into something called a 'Destructor': Destructor (computer programming) - Wikipedia, the free encyclopedia

This will involve implementing a method called '~Linked_list'.

Thank you rod. I have yet to arrive at that point within my text but anticipate it.

You dont call a destructor. It is called automatically when a object goes out of scope or when a 'new'd object is 'delete'd.

Originally Posted by Raigne

You dont call a destructor. It is called automatically when a object goes out of scope or when a 'new'd object is 'delete'd.

True, I understand that but I am calling new in my member function definitions and new in main. When I defined a destructor on Linked_list *link, do I simply let the code go out of scope without calling delete in main? I want to be certain I am not leaking memory when I am ready to completely free my Linked_list.

Originally Posted by laserlight

I suppose I should let you in on the rule of three now: generally, if you define the copy constructor, copy assignment operator or destructor, you should define all three.

The reason is that, unless you are just declaring the destructor to be virtual, the need to define one of them likely means that objects of your class is doing some kind of manual memory (resource) management, hence you have to ask what happens to the resource not only when the object is destroyed, but also when the object is copied. Sometimes, this means that you don't want the object to be copied, in which case you would disable copying by declaring (but not defining) the copy constructor and copy assignment operator as private.

I defined both a copy constructor and assignment operator= including a destructor. My question is how I know for this program that instances of new in member functions and in main results in all used memory returning to the free store or am I dealing with leaks?

Originally Posted by laserlight

By careful examination of your code; by using a memory leak checker, etc. Actually, one of the best ways is to ditch your class and use std::list, but I suspect that that would defeat the purpose of this exercise

Anyway, one of the problems I see is that you are not actually making use of class membership. Consider:

Code:

void Linked_list::display(Linked_list *a_list) const
{
    while(a_list != 0)
    {
        cout << a_list->data << endl;
        a_list = a_list->link;
    }
}

Why on earth do you pass a pointer to a Linked_list to this member function? I would expect something like:

Code:

void Linked_list::display() const
{
    Linked_list *node = link;
    while (node != 0)
    {
        cout << node->data << endl;
        node = node->link;
    }
}

(You may need to fix this for const-correctness.)

Why do you bother to post when you are of no assistance? Your contributions have been the least helpful. I am not asking for a critique of the code but assistance in better understanding and using destructors to ensure I can avoid memory leaks.

Try to respond on topic than with your non-whit, dry humor foolishness. Thank you and have a kind day.

Originally Posted by laserlight

I note that no one mentioned the rule of three before I did. Since my contributions "have been the least helpful", I shall stop helping you after this post. Go ahead and shoot yourself in the foot with C++.

My suggestion to use std::list, while made in jest, really is a way to avoid memory leaks. Refer to Stroustrup's answer to the FAQ How do I deal with memory leaks?


I am giving a critique of your code because you posted code. Pretending that flaws in your code are not relevant won't help you. In fact, making use of class membership is a way to avoid memory leaks: with the unnecessary pointer argument, you are making your code unnecessarily complicated and hence harder to reason about.

Essentially, I see an overall problem with your class design. Take a look at the interface of std::list for an example of one way to approach this that makes use of the Resource Acquisition Is Allocation (RAII) idiom to avoid memory leaks.

I apologize laserlight. I was frustrated, so please forgive me. I rewrote the class and discarded pointer returns, pointer variables, and use of pointers everywhere except to point to another Linked_list. Now I need to implement a smart pointer helper class.

Thank you for your help.