This topic is 3003 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hi: I'll try and be brief. Recently I read again about references in C++ and became interested in using them on my code when I have to pass complete objects onto other methods or functions - I used to do that with pointers -. Still I came with a problem in the ambiguous definitions of some operators... For this case I just wrote a little code simplifying my problem. As you can see I have two + overloaded operators, one for copy, one for reference.
class My_class
{
private:
int member_a;
int member_b;

public:
My_class()
{
member_a=0;
member_b=0;
}

My_class(int a, int b)
{
member_a=a;
member_b=b;
}

~My_class(){ /*Do nothing here*/}

{
return member_a+member_b;
}

void operator=(My_class other)
{
member_a=other.member_a;
member_b=other.member_b;
}

void operator=(My_class & other)
{
member_a=other.member_a;
member_b=other.member_b;
}

My_class operator+(My_class other)
{
My_class temp;
temp.member_a=member_a+other.member_a;
temp.member_b=member_b+other.member_b;

return temp;
}

My_class operator+(My_class & other)
{
My_class temp;
temp.member_a=member_a+other.member_a;
temp.member_b=member_b+other.member_b;

return temp;
}
};

int main(int arg_c, char ** argv)
{
My_class a(1,2);
My_class b(3,4);
My_class c;

c=a+b;
}

/*
cosa.cpp: In function ‘int main(int, char**)’:
cosa.cpp:64: error: ambiguous overload for ‘operator+’ in ‘a + b’
cosa.cpp:39: note: candidates are: My_class My_class::operator+(My_class)
cosa.cpp:48: note:                 My_class My_class::operator+(My_class&)
*/


Of course, there's no way of knowing which one to use... I would just remove the copy operator and there we go: all fine. Now, what if?
c=My_class(1,2)+My_class(3,4);

/*
cosa.cpp:66: error: no match for ‘operator+’ in ‘My_class(1, 2) + My_class(3, 4)’
cosa.cpp:48: note: candidates are: My_class My_class::operator+(My_class&)
*/


Predictable but still stupid... Of course, this should work:
My_class t_alpha(1,2);
My_class t_beta(1,2);

c=t_alpha+t_beta;


Isn't it what I am just doing in the first snippet?. Now, let me be a bit more specific about the ashaming nature of my problem: just for the sake of it I am not using the std classes at all for my "spare time code" - I wouldn't dare to reinvent the wheel on my work time, of course -. It boils down that I just coded my own string (I found it both fun and educative). This string can be constructed from other strings, pointers to char and even integer numbers, so:
My_string a("This");
My_string b(1);
My_string c=a+b;


This works wonders... No matter how long my string is it gets passed by reference when I "add" it to others. But still, this feels kind of stupid:
My_string position;

position=camera.get_x();
position=position+",";
position=position+camera.get_y();

//Now pass it to the screen to be printed...


I should be using...
My_string position=camera.get_x()+","+camera.get_y();
//Now pass it to the screen and do something...


Of course, it just won't compile (using g++, -wno-deprecated) for obvious reasons. So, I am fully aware that I could just be using std::string and do things that way but this happens to echo in my mind as something that could appear as a problem with something else than just a string. I am patient and willing to redo large amounts of the code that uses this "My_string" and, of course, willing to learn. I just don't quite know how to allow implicit reference passing to my methods and still be able to use the syntax I am aiming for... As for me, it seems like a dead end and could not find an answer - could not google for the right question, perhaps? -. So, here's the question: how should I solve the above situation?. As always, many thanks in advance for your time and contributions. Any bit of light, be it a new point of view, be it a possible solution is most welcome.

##### Share on other sites
Constant references.

##### Share on other sites

I happened to read about those in the www.parashift.com site and expected something about them in the replies... Again, I just don't know how to elaborate those in my code.

Just going blind like
My_class operator+(const My_class& other)

Did nothing for me when I tried... Again, that was just running around blind (which I dislike, feels like patching). Could you please elaborate a bit more so I can pick the track?.

Thanks.

##### Share on other sites
class My_class{private:	int a;	int b;public:	My_class() : a(0), b(0) {}	My_class(int a, int b) : a(a), b(b) {}	My_class& operator+=(My_class const& other) {		a += other.a;		b += other.b;		return *this;	}};My_class operator+(My_class const& left, My_class const& right){	My_class temp(left);	return temp += right;}int main(int arg_c, char ** argv){	My_class a(1,2);	My_class b(3,4);	My_class c;	c=My_class(1,2)+My_class(3,4);	c=a+b;}

Eliminated destructor, assignment operator. They are both automatically generated. Implemented operator+ as a non-member function in terms of operator += (increases encapsulation), and then implemented operator+=. Used constructor initializer lists.

##### Share on other sites
Again, many thanks... Studied and tried. I am going to try and implement it in my real code to see if I can get something done...

Still, there's a doubt here:

My_class operator=(My_class const& other)	{		member_a=other.member_a;		member_b=other.member_b;	}

Why not?

void operator=(My_class const& other)	{		member_a=other.member_a;		member_b=other.member_b;	}

I don't see the = operator returning any value, so, why setting the return type to My_class?.

On a side note, for those who mind find this useful, adding:

void operator=(My_class other)	{		member_a=other.member_a;		member_b=other.member_b;	}

Brings back the problem but:

void operator=(My_class & other)	{		member_a=other.member_a;		member_b=other.member_b;	}

Keeps things on track: I guess the static reference should keep everything from mutating my "other".

I will try and keep the results updated, just in case it happens to help someone with the same problem.

Thanks a lot.

Edit: didn't catch your edit in time. I am through it now.

##### Share on other sites
The assignment operator returns what was assigned to. If it doesn't then you run into the issue of (a = b).c();, or more commonly: a = b = c;.

More importantly, if you don't require specialized behavior in your assignment operator, then avoid implementing it. The automatic one will do pretty much anything your trivial one will do.

The same is also true of copy constructors.

Also of note: If you have to implement the assignment operator then you should generally implement the trio-of-despair: Copy constructor, assignment operator, and destructor.

##### Share on other sites
I see the a = b =c point... As for the assignment operator, I felt I should implement it (as I did with the real copy constructor) since I am dealing with memory allocation in my real case. I read somewhere that the default methods will do bitwise copies and I would end up with two different instances pointing at the same memory location, which is not desirable at all I guess...

As for the real problem, still trying to figure it out:

Cadena Cadena::operator+(Cadena const& other){	char * temp_pointer=concat(pointer_to_char, other.get_pointer_to_char());	Cadena temp(temp_pointer) //This would copy temp_pointer contents to the pointer_to_char of temp...	delete(temp_pointer);	return temp;	}/*error: passing ‘const Cadena’ as ‘this’ argument of ‘char* Cadena::acc_cadena()’ discards qualifiers*///Edited to translate the code from the original spanish.

I am not sure if I will ever be able to do what I aim for:

Cadena my_string=camera.get_x()+","+camera.get_y()+" some other thing";

Since camera.get_x()+","+camera.get_y()+" some other thing" doesn't seem to evaluate to something that any of my assignment operators would expect.

I can always "prefix" like:

[source source="cpp"]Cadena my_string=Cadena(camera.get_x())+Cadena(",")+Cadena(camera.get_y())+Cadena(" some other thing");

And keep going until it is readable no more... Still, I'll have to figure the "discards qualifiers" problem first.

##### Share on other sites
Quote:
 Original post by The_Marlboro_Manerror: passing ‘const Cadena’ as ‘this’ argument of ‘char* Cadena::acc_cadena()’ discards qualifiers

You're calling a non-constant member function on a constant member function pointer/reference. That's not allowed.

If you're using pointers, well first off, as is usually pointed out: use std::string. It exists, it works, it does most of what you want, and the other stuff that it does do is a hell of a lot safer to write for std::string than for character pointers.
If it's not a string, then use vectors. again, automatic memory handling will decrease the number of leaks and other issues you have. Not to mention, chances are you're implementing your copy constructor, assignment operator, or destructor wrong. Which will either result in leaky code, or worse.

##### Share on other sites
Thanks again. I will - of course - use the std utilities for anything that is meant to be serious (or at least, working) but I usually find that messing around - reinventing the wheel, as we call it - helps me understand certain concepts of the language (as well as gets me to terrible mistakes, but well, mistakes are part of my formative self).

That is, I can always use safe code for safe results but in this particular case I am trying to get a bit deeper into this wall I am stuck against. Of course, I appreciate your comments and take them into consideration: if I ever happen to release something I do with this code I will make sure to replace all my Cadena's with std::string's: the point for me to release code is to make it readable for others... As far as I am concerned I am done with my Cadena when I finally get everything I want of it.

Just to make it clear, I sincerely appreciate your comments and interest (and, of course, the time you're taking to read and write here).

Switching topics and now that you mention it, all this started from repairing a memory leak. It seems to be working properly now - at least some abusive tests seem to point that out - even though I am not using the this+that+another part.

As for the "passing ‘const Cadena’ as ‘this’ argument of ‘char* Cadena::acc_cadena()’ discards qualifiers" I seem to be making some progress but I am also getting lots of error bumps in other files... But I better do some more research before I attempt something wrong.

##### Share on other sites
Personally, I think that if you want to reinvent the wheel you should do it in its own project where the sole purpose is to just reinvent the wheel for that one thing. I wouldn't mix your reinvention with the idea that you'll just replace it later because that almost never happens.

##### Share on other sites
The conventional signature for an overloaded assignment operator usually looks like this
My_class& operator=(const My_class& other)

instead of returning nothing (void), you can return a reference to the current object, to allow chaning of the = operator, e.g. a=b=c=d;
   member_a=other.member_a;   member_b=other.member_b;   return *this;

As for the + operator, you could consider changing that to pass-by-const-reference too (It won't be modifying the existing object so it can be a const member function)
 My_class operator+(const My_class& other) const

const correctness is a good thing to get right from the outset, although if you're already in the middle of a project (especially one which hasn't been designed with it in mind), trying to add const all over the place to existing classes can sometimes be a headache.

##### Share on other sites
Hi:

nobodynews, you absolutely have the point there... As I will comment later on this post, I ended up with the "snowball" effect and soon I will be done with my particular implementation of the wheel so I can start new with something more standard... Actually, now that I think about it I started using this - buggy - implementation of a string because I had been working with it lately and felt familiar with it. By the way, memory leaks appeared again in the addition operator, shall take a look at it shortly.

Bench, I ended up going to bed at 2:00am yesterday just because "const correctness" (not a good thing to say about someone responsible at work but well, the ocassion deserved it). I happened to be more and more interested and started to evaluate my header files and considering "constness" for each single thing that shouldn't change... As you may imagine, one constant reference led to other, and other and other and ended up snowballing all around my code.

I wonder, is that a good thing at all?. I was forced to use the const_cast in a few times at the end of the chain because the SDL library doesn't have a prototype for SDL_BlitSurface(const SDL_Surface * [...]) but for (SDL_Surface * [...]) and I am not sure at all about all those "const" and "&" around my code.

Perhaps I should talk a bit about how I think it works... Let's see I am working at the graphic functions right now. I consider a few "entities":

-> Resources, surfaces loaded with an image and also a string with the file name.
-> Drawing blocks -> Structures with a pointer to a resource (what to draw) and to other SDL structures that point "Where" to draw (X, Y...). Also some other values for "draw depth" and offsetting of the resource.
-> The screen -> The entity that takes all the drawing blocks and puts them on the screen.

It's actually a bit larger than that, but that should do it.

So, my resources have to be passed to the drawing blocks. Since I don't want the drawing block to modify the original resource in any way the resource just returns a constant pointer to its surface (SDL Surfaces work with pointers...).
Same with the filename, of course.

Then, my drawing blocks had to be modified so they take a constant resource on their constructor... Some stuff I had to access into the resource when building the block had also to be made "const" - no changes made to the data - so I could call it. I also had to modify some code inside the blocks themselves since I wanted them to be passed to the screen as "const" too.

And of course, the screen has to get the blocks as constants since I don't want the screen to modify anything, just to draw. Here it came the const_cast bit: at the end of the chain, the original pointer to an SDL_Surface in the resource has become constant and SDL won't allow them as parameters so, in the end of the chain I have to cast them and pray the SDL does nothing to them (why this does seem so wrong?).

So, well, there are more and more ramifications of this around my code (I just copied and pasted the project so I woudn't break everything) and I am wondering if I am doing this right. So far I am trying to "normalize" the parameters to my methods so they take constant references (or references where there's no need for a constant) and also tracing the chain of my code to determine what things shouldn't be changed and make those constant references. Am I right or wrong about this?. Honestly, I am so absolutely new to all this constness and I would like to know more about it.

Thanks a lot.

##### Share on other sites
Quote:
 I wonder, is that a good thing at all?. I was forced to use the const_cast in a few times at the end of the chain because the SDL library doesn't have a prototype for SDL_BlitSurface(const SDL_Surface * [...]) but for (SDL_Surface * [...]) and I am not sure at all about all those "const" and "&" around my code.
Yes, const-correctness is worth the trouble (IMO). And yes, if you have a large project that isn't consistently const-correct and you decide you want to make it const-correct, you can end up with a domino effect that can take a while to sort out. And yes, sometimes you may have to use const_cast to interoperate with third-party libraries that are not const-correct themselves (IMX, at least).