My problem with a pointer in a template class

[Kubani]

Hello all,
The code below is part of greater program. Please have a look at it.

#include <iostream>
using namespace std;

class god {
public:
	god(string n, string m) : name(n), mythology(m) {}
	void god_print_all() { cout << name << "  " << mythology << endl; }
	string get_name() { return name; }
private:
	string name, mythology;
};

//-----------------------------------------------------

template <class T> class Link {
public:
	Link(const T& v, Link* p = 0, Link* s = 0): g1(v), value(v.get_name()), prev(p), succ(s) { }
	
 Link* insert(Link*);  // OK
 Link* next() const { return succ; }  // OK
 Link* previous() const {return prev; }  // OK
 void get_value() { g1.god_print_all(); }
    
private:
	god g1;
	string value;
	Link* prev;
	Link* succ;
};

//---------------------------------------------------------------------------------------------------------------

template <class T> Link<T>* Link<T>::insert(Link<T>* n)  //insert n before "this", return n
{
	if(n == 0) return this;
	if(this == 0) return n;
	n->succ = this;
	if(prev) prev -> succ = n;
	n -> prev = prev;
	prev = n;
	return n;
}

//-----------------------------------------------------------------

template <class T> void link_print_all(Link<T>* p)
{
    cout <<"{ ";
	while(p) {
		  p -> get_value();
		if(p = p -> next()) cout << ", ";
	}
	cout << " }\n";
}

//-----------------------------------

int main() 
{
	Link<god>* norse_gods = new Link<god> (god("First","Second"));
	norse_gods = norse_gods -> insert(new Link<god>(god("One","Two")));

	cout << "norse_gods are: "; link_print_all(norse_gods);
	cout << "\n";
	
	return 0;
}

value (inside class Link) is a string and the return type of v.get_name() is also a string. This function returns the string name inside the class god.

And that string (name) goes into the value inside class Link.

In Link(const T& v, Link* p = 0, Link* s = 0, v is a const reference to T and T is a god here, but we don't want to change it. We want to copy something from it and paste it into value (of the class Link).
Every thing appears fine but, I get the following error!

Error 1 error C2662: 'god::get_name' : cannot convert 'this' pointer from 'const god' to 'god &' 15

I tried much to figure out why the compiler gives this error while everything is fine, but I have not been able to, so far.

Would you please help me?

[Peter87]

You have forgot to mark the get_name() function as const.

string get_name() const { return name; }

[Kubani]

OK, but what does that const in string get_name() const { return name; } mean please?

[TheIdeasMan]

When I compile your code with cpp.sh, with all warnings on, I get these errors

In instantiation of 'Link<T>::Link(const T&, Link<T>*, Link<T>*) [with T = god]':
52:62: required from here
15:89: error: no matching function for call to 'god::get_name() const'
15:89: note: candidate is:
8:9: note: std::string god::get_name() <near match>
8:9: note: no known conversion for implicit 'this' parameter from 'const god*' to 'god*' In instantiation of 'void link_print_all(Link<T>*) [with T = god]': 55:55: required from here
45:3: warning: suggest parentheses around assignment used as truth value [-Wparentheses]

Making the function const as suggested by the warning, I get this:

In instantiation of 'void link_print_all(Link<T>*) [with T = god]':
55:55: required from here
45:3: warning: suggest parentheses around assignment used as truth value [-Wparentheses]

So for your const correctness foo, any function that doesn't change the sate of the class should be const. Arguments can quite often be const as well.

Rather than set pointers to 0, use nullptr instead.

Try not to use naked pointers at all, consider smart pointers instead.

[Peter87]

Kubani wrote:

OK, but what does that const in string get_name() const { return name; } mean please?

It means that the function promise not to modify the object.

v in the Link constructor has been marked as const. That means you are not allowed to modify v, and that's why you are only allowed to call member functions marked as const.

You should get into the habit of using const for all member functions that doesn't modify the object they are called on. That means god_print_all() should also be const, because it's just reading the variables of the god object without modifying them.

[Kubani]

Thank both of you. The change made the code run successfully.
But for more info and being more familiar with these subjects, I'd like to ask some more questions.

1- Without that change if I run the code using the possibility of the site (that small button), the errors I get are different from yours TheIdeasMan. I don't get that 15:89 error!

2- I reran the code using cpp.sh. It ran and as you said gave me a warning:
51:3: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
It refers to the line 51 of the code. Does that warning mean I had better put parentheses around p = p -> next() assignment inside the if condition?

any function that doesn't change the sate of the class should be const

Both god_print_all() and get_name() are god's member function and both don't change the state of the class god. But not making the god_print_all() const doesn't cause any problem!

And also about get_name(), although it's called by a const reference (v) but it doesn't change any thing in the state of the class (god) so logically not making this one, too, const, should not cause any problem. Don't you agree?

[MikeyBoy]

But not making the god_print_all() const doesn't cause any problem!

Sure. There's no requirement to make a method const, even if it doesn't change the state of the object. But it's a helpful thing to do.

[Peter87]

Kubani wrote:

Does that warning mean I had better put parentheses around p = p -> next() assignment inside the if condition?

It is a common error to use = instead of == in if statements so compilers often warn about this. If you want to assign a variable inside an if statement you can put an extra pair of parentheses around the expression to make the warning go away.

if ((p = p->next()))

[Kubani]

Thank all of you nice guys very much.

MikeyBoy wrote:

Sure. There's no requirement to make a method const, even if it doesn't change the state of the object. But it's a helpful thing to do.

OK. I did it too for the function god_print_all() but if we don't make get_name() const, what part of the class god will be affected (changed) in v by value(v.get_name())?
I know we should make get_name() const, but I don't know what its reason is. That function does not attempt make a change in const v.

[Peter87]

When you call a function on a const object the compiler will not look inside the function to see if the function is modifying the object. You (the programmer) has to mark the function as const in order to tell the compiler that the function is safe to call on const objects.

[TheIdeasMan]

Kubani wrote:

Without that change if I run the code using the possibility of the site (that small button), the errors I get are different from yours TheIdeasMan. I don't get that 15:89 error!

Did you turn on all 3 warnings? One should set the warning level to it's highest, warnings are a good thing: they tell you about potential problems in your code. As a minimum with g++ or clang++, use -Wall -Wextra -pedantic. There are some who make all the warnings errors (-Werror), and even pedantic warnings (-pedantic-error). This is because one really isn't finished until there are no warnings at all.

It's worth reading the compiler manual, there are some handy warnings, not turned on by even the options already mentioned. I know there are a zillions lines in the manual, but it's still worth it IMO.

const correctness is a huge help too, it can help you to not mess things up.

[MikeyBoy]

As Peter87 said, if you have a const object, the compiler will only allow you to call const methods of that object. So you won't be able to call get_name() on a const object, unless you make get_name() const.

[Kubani]

Yes guys. Thanks to all of you.

@TheIdeasMan: You are right. I turned on those options and they gave me what I expected. Thanks also for your explanations. Programming is a very huge and subtle subject. Now I can understand why experts say: "10 years is needed to be a real programmer (in a one like C++)"!

@Peter: The reason you (and MikeyBoy) mentioned is fine and acceptable. As usual, your answers are very precise. :) A special thanks to you. Thanks MikeyBoy.