Archived

This topic is now archived and is closed to further replies.

CArray Craziness

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

If you intended to correct an error in the post then please contact us.

Recommended Posts

Alright, I''m trying to use the MFC CArray class with my own class, called CDayRecord. I got errors saying I had to make a copy constructor and an overload of operator=. Now I get a warning, "returning address of local variable or temporary". If someone would check my code, I''d appreciate it. // Data Representation class CDayRecord { public: CTime Day; CStringArray m_saItemDesc; CArray m_aMtiData; CArray m_naFoodPoints; unsigned int m_nExercisePoints; float m_fWeight; // Functions CDayRecord(); CDayRecord(const CDayRecord &rhs); CDayRecord & operator=(const CDayRecord &rhs); void Clear(void); }; CDayRecord::CDayRecord(const CDayRecord &rhs) { Day = rhs.Day; m_fWeight = rhs.m_fWeight; m_nExercisePoints = rhs.m_nExercisePoints; for(int i=0; i < rhs.m_aMtiData.GetSize(); i++) { m_aMtiData.Add(rhs.m_aMtiData); } for(i=0; i < rhs.m_saItemDesc.GetSize(); i++) { m_aMtiData.Add(rhs.m_aMtiData[i]); } for(i=0; i < rhs.m_naFoodPoints.GetSize(); i++) { m_aMtiData.Add(rhs.m_naFoodPoints[i]); } } // This is the part that''s wrong CDayRecord & CDayRecord::operator=(const CDayRecord &rhs) { CDayRecord c; c.Day = rhs.Day; c.m_fWeight = rhs.m_fWeight; c.m_nExercisePoints = rhs.m_nExercisePoints; for(int i=0; i < rhs.m_aMtiData.GetSize(); i++) { c.m_aMtiData.Add(rhs.m_aMtiData[i]); } for(i=0; i < rhs.m_saItemDesc.GetSize(); i++) { c.m_aMtiData.Add(rhs.m_aMtiData[i]); } for(i=0; i < rhs.m_naFoodPoints.GetSize(); i++) { c.m_aMtiData.Add(rhs.m_naFoodPoints[i]); } return c; } Thanks for your help

Share this post


Link to post
Share on other sites
quote:
Original post by NorthWoodsman
...I get a warning, "returning address of local variable or temporary". If someone would check my code, I'd appreciate it.
  
CDayRecord & CDayRecord::operator=(const CDayRecord &rhs)
{
CDayRecord c;
// *Snip* copying from rhs to c
return c;
}

It seems you've got a little mixed up with how operator= is supposed to work. It doesn't create a new object, copy the contents from rhs and return a reference to the new object. The statement a = b effectively becomes a.operator=(b), so you copy the contents of b into a, returning a reference to a. The reference return allows chaining, thus:

a = b = c; // Assign c to b, then assign b to a

so your operator= needs to operate more like this:


  
// Return a const reference to permit chaining

const CDayRecord & CDayRecord::operator=(const CDayRecord &rhs)
{
// Prevent self-assignment

if(this != &rhs)
{
Day = rhs.Day;
// etc...

}

return *this; // return a reference to the lhs

}

The self assignment check prevents a=a from corrupting a. If it wasn't there the assignment operator might dispose of the contents of a.m_aMtiData (and the other CArrays). Then when it tried to copy the contents of rhs (a reference to a) they are gone because it just disposed of them. One way to prevent this is to make sure that the address of the lhs (this) and the rhs (&rhs) are not the same before doing anything.

As for the warning the compiler is giving you, that is something you should pay attention to. You're currently creating a local variable and returning a reference to it. When the end of the fuction is reached, c gets destroyed because it has gone out of scope. Unfortunately that means you are returning a reference to an object which no longer exists, which means any use of the reference is a very bad idea. Fixing the operator= as above will get rid of that problem though.

PS: if you're posting source code, use the [ source ] [ /source ] tags (without the spaces) to preserve the formatting.

[edited by - Krunk on August 29, 2002 3:48:38 PM]

Share this post


Link to post
Share on other sites