should i use initialization list?

[manav]

#include <iostream>

template <typename Type>
class LinkList {
	struct Node {
		Type item;
		Node *next;

// Should i use initialization list here? 
// Which is better? (safer? faster?)
		Node(Type i, Node *n=0) {
			item = i;
			next = n;
		}
	} *root;
	
public:
	LinkList() : root(0) {}

	void add(Type item) {
		if (root) {
			Node *tmp = root;
			while (tmp->next) tmp = tmp->next;
			tmp->next = new Node(item);
		} else {
			root = new Node(item);
		}
	}

	void print() {
		std::cout << "List items: ";
		for (Node *tmp = root; tmp; tmp = tmp->next) {
			std::cout << tmp->item << " ";
		}
		std::cout << std::endl;
	}	
};

int main()
{
	LinkList<int> list;
	int choice, item;
	while (true) {
		std::cout << "1) Add\n2) Print\n0) Quit\n? ";
		std::cin >> choice;
		if (choice == 1) {
			std::cin >> item;
			list.add(item);
		} else if (choice == 2) {
			list.print();
		} else if (choice == 0) {
			break;
		}
	}
	
	return 0;
}

[laserlight]

Should i use initialization list here?

Yes.

Which is better? (safer? faster?)

Using an initialization list is potentially faster since it avoids creating the member object and the assigning to it.

[brewbuck]

// Should i use initialization list here?
// Which is better? (safer? faster?)

Add "More correct" to that list.

If you don't list a member in the initialization list, it will be default-constructed. Then in the constructor body, it will be assigned a new value through its assignment operator. For one thing, this is inefficient, for another, it is simply wrong if the member has no default constructor or if its constructor does something different from what the assignment operator does.

For basic data types it doesn't matter as much, but the rule is to use the initialization list to initialize members unless there is some reason why that is completely impossible.

[manav]

thanks laserlight, brewbuck.
i understand now.

[manav]

this is ~destructor for the LinkList class:

~LinkList() {
		Node *tmp = root;
		while (tmp) {
			delete tmp;
			tmp = tmp->next;
		}
	}

is it working correctly? i mean releasing the memory properly?

[brewbuck]

is it working correctly? i mean releasing the memory properly?

It's wrong. You're accessing an object after it has been destructed.

[manav]

It's wrong. You're accessing an object after it has been destructed.

sorry, i am confused, how is it wrong?

[brewbuck]

sorry, i am confused, how is it wrong?

You delete tmp, and then you access tmp->next. You shouldn't be accessing an object which doesn't exist.

[manav]

sorry i skipped that one in a hurry, after your post i rechecked my code, strange thing is that there was no runtime error during 3 test runs.

now corrected:

~LinkList() {
		while (root) {
			Node *tmp = root->next;
			delete root;
			root = tmp;
		}
	}

is it ok now?

[brewbuck]

sorry i skipped that one in a hurry, after your post i rechecked my code, strange thing is that there was no runtime error during 3 test runs.

That sort of error is unlikely to cause a runtime fault... at the time.

In a bigger program, errors like this eventually lead to instability and a crash.

~LinkList() {
		while (root) {
			Node *tmp = root->next;
			delete root;
			root = tmp;
		}
	}

is it ok now?

Looks good now.

[matsp]

Also, if your Node is a class which has a destructor like this:

~Node() 
{
   next = 0;
}

then you would only delete the first item...
Or:

~Node() 
{
   next = reinterpret_cast<Node *>(-1);
}

would almost certainly make it crash.


--
Mats

[manav]

thanks Mats.
why would that happen is not clear to me, still, then what to do so that the LinkList is properly destroyed and all memory is freed?

[matsp]

thanks Mats.
why would that happen is not clear to me, still, then what to do so that the LinkList is properly destroyed and all memory is freed?

I am talking about in your broken case where you access the node after it's been deleted. Try it if you like.

--
Mats

[manav]

I am talking about in your broken case where you access the node after it's been deleted. Try it if you like.

thanks again.
actually adding a destructor to Node struct would have produced run time error, and, would have made me catch this earlier, i think my Node struct must have a destructor.