Advertisement

Copy Constructor and Uninitialized this

Started by October 02, 2000 01:16 PM
8 comments, last by Orpheum 24 years, 3 months ago
Another problem with operator overloading... I overloaded the = operator for my BitmapFile class, but when the flow enters operator= scope, this is uninitalized, and thus assigning any values causes Access Violations. I''m new to copy constructors, so its quite possible I''m not doing something right, but from looking at my book, I can decern what it is. Here''s some code:
    
BitmapFile::BitmapFile(const BitmapFile& copy)
:BitmapID(copy.BitmapID)
{
   bmfh	 = copy.bmfh;
   pbmi	 = (LPBITMAPINFO)LocalAlloc(LPTR, sizeof(BITMAPINFO));
   memcpy(pbmi, copy.pbmi, sizeof(BITMAPINFO));

   if(copy.pbmih->biBitCount == 8)
   {
      (LPBITMAPINFO)LocalAlloc(LPTR, sizeof(BITMAPINFO) + (255 * sizeof(RGBQUAD)));
      memcpy(&pbmi->bmiColors[1], &copy.pbmi->bmiColors[1], (255 * sizeof(RGBQUAD)));
   }
		
   pbmih	 = &pbmi->bmiHeader;
   iNumBits = copy.iNumBits;
	
   buffer	 = new UCHAR[iNumBits];		
   memcpy(buffer, copy.buffer, iNumBits);
}


BitmapFile& BitmapFile::operator=(const BitmapFile& source)
{
//dont do self-assignment

if(&source != this)
{
   bmfh	 = source.bmfh;
   pbmi	 = (LPBITMAPINFO)LocalAlloc(LPTR, sizeof(BITMAPINFO));
   memcpy(pbmi, source.pbmi, sizeof(BITMAPINFO));

   if(source.pbmih->biBitCount == 8)
   {
      (LPBITMAPINFO)LocalAlloc(LPTR, sizeof(BITMAPINFO) + (255 * sizeof(RGBQUAD)));
      memcpy(&pbmi->bmiColors[1], &source.pbmi->bmiColors[1], (255 * sizeof(RGBQUAD)));
   }
		
   pbmih = &pbmi->bmiHeader;
   iNumBits = source.iNumBits;

   buffer	 = new UCHAR[iNumBits];		
   memcpy(&buffer, &source.buffer, iNumBits);
}

return *this;
}
    
I''ve never really used either of these methods very much, only in class really... my usual style is sort of a C and C++ hybrid I call C+. =) I''m trying to quit being lazy and get back to real C++ (because being lazy makes more work in the end!!).
There is no spoon.
Ok. Maybe I''m missing something here... why not do this?

    BitmapFile& BitmapFile::operator=(const BitmapFile& source){  (*this) = BitmapFile(source);  return (*this);}    


I mean, with the exception of the if() statement, looks like both functions are doing the same thing anyway...

I''m fairly new to this copy constructor stuff too, so that''s why I am asking.

Regards,
Jumpster





Semper Fi
Regards,JumpsterSemper Fi
Advertisement
Umm.... because this is const?
Actually, all you do, is to do the operator= in the copy constructor, and do all the work in the operator=.

------------------------------
BCB DX Library - RAD C++ Game development for BCB
You know I thought the whole copy constructor/operator= thing was funny too... in the book it looks like they are doing the same thing, so thats pretty much what I did! The problem doesn''t lie within the copy constructor because I commented all the guts out of it and I still get the access violation! Maybe I''m overwriting a pointer or some shit... I''m out of ideas for the moment (I know this will come to me someday when I''m taking a crap or something... but hopefully someone will have the answer before that!).

Let me ask this... is what I''m doing in the copy constructor, and then re-doing in operator=, "standard procedure"? I thought the authors of my book were a little off-kilter after reading the example, but I''m no expert! I want to know what the real experts think! If anyone out there knows of a good tut on overloading, please point that out too! One can never have enough sources of info... I will go look for one now too. Thanks!!
There is no spoon.
Jumpster - The reason you cant do a member-wise copy is that when you have dynamically allocated memory, you end up with both objects pointing to the same piece of mem. When the first destructor is called, the other object will have a bogus pointer.
There is no spoon.
The standard operator= will make a prefect copy of your object for the new one to be equal too. This works perfectly fine until you have a pointer in your class. If you didn;t overlaod the = operator then two object would point to the same stuff in memory (which is usually realy realy bad, xsp at destruction time)

So in the = op you need to assign all the normal properties and make new copies of the stuff pointed to by your pointers to assign the new instance of your class to.

The copy constructor is used when you make a new class like this:
CMyClass test;
CMyClass* test2 = new CMyClass(test); //copy constructor called

and has the same problems as the assignment with regard to pointers.

and typically you call the =op from the copy constructor.

and you don''t have to create a copy constructor and assignment operator - where are you copying or assigning bitmaps to eachother? You should be using a handle or a pointer to it.
- The trade-off between price and quality does not exist in Japan. Rather, the idea that high quality brings on cost reduction is widely accepted.-- Tajima & Matsubara
Advertisement
I understand that about the pointers to memory are bogus, but you have redefined the copy constructor already. It is creating all new memory allocations and copies the memory from the source to the new object (LocalAlloc, memcpy).

Jumpster

Semper Fi
Regards,JumpsterSemper Fi
quote:
Ok. Maybe I''m missing something here... why not do this?...


The line: (*this) = BitmapFile(source); will call BitmapFile::operator= with a temp BitmapFile copy constructed from ''source''. So basically your calling operator= inside operator=, which isn''t want you wanted. If you only want to have the code in one place you should call operator= from the copy constructor and put the needed stuff in there.
Thanks guys... I thought that what they were doing was redundant, but again, this isn''t my strongest area of C++. I got everything working with this duplicate setup... I will try to tinker with it when I get home by placing a call to operator= in the copy constructor.
There is no spoon.
Thanks Wilka, I appreciate it. Now I understand why that is.

Regards,
Jumpster



Semper Fi
Regards,JumpsterSemper Fi

This topic is closed to new replies.

Advertisement