Functions returning my user-defined class (w/pointer) don't like operator=.

[oliveman]

I have a user-defined class containing several integers and a pointer (to an array of values in another user-defined class which does not contain a pointer, and which works fine and has no similar problems) as data elements. I am using g++.

My problem is this:

If I take a function returning my class, e.g.

myClass myFunction()

and attempt to either set a variable equal to it or return it, e.g.

int main()
{
myClass A;
A=myFunction();
}

or

myClass anotherFunction()
{
return myFunction();
}

I get a compiler error from operator=/the copy constructor, respectively. I have overloaded both of these.

The error is that it seems to call myClass(myClass) instead of myClass(myClass&) for the copy constructor and operator=(myClass) instead of operator=(myClass&), and obviously it doesn't like that. I assume the problem is that things are stored at a particular address only temporarily (until the function finishes), but I could easily be wrong.

Why does this happen and how can I fix it?

Thanks!

[TheCPUWizard]

1) Please use code tags. Go back and re-read the "BEFORE you post announcement if you have forgotten how.

2) Please post minimal yet complete code. This means the smallest possible sample that someone can copy/paste into an empty file and run through the compiler to reproduce the exact issue (error line number should line up!

[Paul McKenzie]

Why does this happen and how can I fix it?

My crystal ball says that you have a bug on line 57.

Seriously, how can we fix an error if we don't see the class that's causing the problem?

Regards,

Paul McKenzie

[TheCPUWizard]

My crystal ball says that you have a bug on line 57.

Seriously, how can we fix an error if we don't see the class that's causing the problem?

Regards,

Paul McKenzie

A bug in 57 would be quite upsetting

[oliveman]

Here is a class which produces the errors. With this code:

class myClass
{
public:
myClass();
int Read(int k);
myClass(myClass & original);
myClass & operator=(myClass & original);
~myClass();
friend myClass myFunction();
friend myClass anotherFunction();
private:
int * ptr;
};
int myClass::Read(int k)
{
return ptr[k];
}
myClass::myClass()
{
ptr=new int[5];
}
myClass::myClass(myClass & original)
{
int k=0;
while (k<5)
	{
	ptr[k]=original.Read(k);
	k++;
	}
}
myClass & myClass::operator=(myClass & original)
{

int k=0;

while (k<5)

	{

	ptr[k]=original.Read(k);

	k++;

	}

return *this;

}
myClass::~myClass()
{
delete [] ptr;
}
myClass myFunction()
{
myClass A;
return A;
}
myClass anotherFunction()
{
return myFunction();
}
int main()
{
myClass A;
A=myFunction();
A=anotherFunction();
}

I get these compiler errors:
./file.cpp: In function ‘myClass anotherFunction()’:
./file.cpp:52: error: no matching function for call to ‘myClass::myClass(myClass)’
./file.cpp:22: note: candidates are: myClass::myClass(myClass&)
./file.cpp: In function ‘int main()’:
./file.cpp:57: error: no match for ‘operator=’ in ‘A = myFunction()()’
./file.cpp:31: note: candidates are: myClass& myClass:perator=(myClass&)
./file.cpp:58: error: no match for ‘operator=’ in ‘A = anotherFunction()()’
./file.cpp:31: note: candidates are: myClass& myClass:perator=(myClass&)

[TheCPUWizard]

You are attempting to take a non-const reference of a temportary.

                int Read(int k) const;
	myClass(myClass const & original);
	const myClass & operator=(myClass const & original);

You need to do some detailed studying on what "const correctness" means....

[Paul McKenzie]

Here is a class which produces the errors.

In addition to what CPUWizard points out, your code has other runtime errors. Also, you need to format your code a little better, since too much of it is not indented properly and is hard to read.

First, usage of std::vector eliminates the need for copy constructor and assignment operator. However, here is your mistake:

myClass::myClass(const myClass & original)
{
    int k=0;
    while (k<5)
    {
        ptr[k]=original.Read(k); // Error!  Invalid memory access 
        k++;
    }
}

See the line in red? It is wrong, since ptr was not allocated.

The fix to both the default and copy constructor is to make sure that ptr is allocated on initialization of the object. You would use the initializer list instead of allocating it within the constructor body.

myClass::myClass() : ptr(new int[5])
{ }

myClass::myClass(const myClass & original) : ptr(new int[5])
{
    k = 0;
    while (k<5)
    {
        ptr[k]=original.Read(k);
        k++;
    }
}

Regards,

Paul McKenzie

[TheCPUWizard]

Paul,
I only made sure it would compile, not that it was logically correct