Help With Copy Constructor.

[JoeC]

Re: Help With Copy Constructor.

It is a complex problem I am working on. Still thanks for the help.
The best I can do right now is get the graphic to show garbled. So
some where I a loosing the graphics data. Thanks for your time.

[JoeC]

Re: Help With Copy Constructor.

I have been working on this problem for a while and still my program is
getting better. The object seems to work but it looks like my graphics
information is getting corrupted.

What I do is create the graphic
Then I change the graphics information
finlaay I use the copy constructor.


Here is the code:

graphic::graphi c(){
lr = ud =16;
BYTE c[32] = {0xff,0xff,0xff ,0xff,0xff,0xff ,0xff,0xff,
0xff,0xff,0xff, 0xff,0xff,0xff, 0xff,0xff,
0xff,0xff,0xff, 0xff,0xff,0xff, 0xff,0xff,
0xff,0xff,0xff, 0xff,0xff,0xff, 0xff,0xff};

copy(c);
BITMAP bitmap = {0,ud,lr,2,1,1} ;
bitmap.bmBits = gdata;
hbitmap = CreateBitmapInd irect(&bitmap);
}

void graphic::set(co nst BYTE c[]){
copy(c);
ud = lr = 16;
BITMAP bitmap = {0,ud,lr,2,1,1} ;
bitmap.bmBits = gdata;
hbitmap = CreateBitmapInd irect(&bitmap);
}


graphic::graphi c(const graphic& gr){
ud = lr = 16;
//memcpygdata, gr.gdata, 32 );
for(int lp = 0; lp != 32; lp++){
gdata[lp] = gr.get(lp);}
BITMAP bitmap = {0,ud,lr,2,1,1} ;
bitmap.bmBits = gdata;
hbitmap = CreateBitmapInd irect(&bitmap);
}

[Dennis Jones]

Re: Help With Copy Constructor.


"JoeC" <enki034@yahoo. com> wrote in message
news:1147218539 .959800.266680@ g10g2000cwb.goo glegroups.com.. .[color=blue]
> I have been working on this problem for a while and still my program is
> getting better. The object seems to work but it looks like my graphics
> information is getting corrupted.
>
> What I do is create the graphic
> Then I change the graphics information
> finlaay I use the copy constructor.[/color]

<snip>

If 'gdata' is a vector (which I think you stated in one of your previous
messages), you can simplify things significantly. If 'gdata' is not already
a vector, consider making it a vector so that you can do this (note the
simplicity -- no 'copy' loop):

graphic::graphi c(const graphic& gr)
{
...
gdata = gr.gdata;
...
}

Okay, now let's refactor things a little. I think it would be good to
factor out the bitmap creation code into its own function, because it is
getting duplicated in several places:

HBITMAP graphic::create Bitmap( void )
{
BITMAP bitmap = { 0, ud, lr, 2, 1, 1 };
bitmap.bmBits = &gdata[0];
HBITMAP hBitmap = CreateBitmapInd irect(&bitmap);
return hBitmap;
}

Now IMO, an "ideal" copy constructor would perform a _simple_ assignment of
as many data members as it possibly can, only performing more complex copy
operations if/when it is absolutely necessary. For instance, obviously, you
can't do a simple assignment with pointer members (such as hbitmap), but you
can with most other members (assuming they support an assignment operator,
like vector does). So, I think I would probably write the copy-constructor
like this:

graphic::graphi c(const graphic& other)
{
ud = other.us;
lr = other.lr;
gdata = other.gdata;
hbitmap = createBitmap();
}

You don't need the "copy" function. Assuming you make 'gdata' a vector, a
simple assignment will work just fine (and it makes the code easier to read
and understand and more robust in the face of change).

I'm not sure why you have a "set" method, since it pretty much does the same
thing as the copy-constructor, but if you need to keep it for some reason, I
would change the 'c' argument to be a vector, like this:

void graphic::set( const std::vector<BYT E> &c )
{
gdata = c;
hbitmap = createBitmap();
}

And your default constructor becomes:

graphic::graphi c()
{
ud = lr = 16;
for ( int i=0; i<32; ++i )
{
gdata.push_back ( 0xFF );
}

hbitmap = createBitmap();
}

I hope this makes sense to you and provides you with some direction.

- Dennis

[JoeC]

Re: Help With Copy Constructor.

Thanks for the time in explaining how that works.

That helped, ithink my graphic object is much better but it still
crashes at this point:

graphic::graphi c(const graphic& gr){
ud = lr = 16;
//gdata = gr.gdata; <-- This crashes
BITMAP bitmap = {0,ud,lr,2,1,1} ;
bitmap.bmBits = &gdata[0];
hbitmap = CreateBitmapInd irect(&bitmap);
}

That bity of code crashes when I use the command like this:

if(play){
cgr = new graphic(play->gOut());
return *cgr;
--------------
graphic gOut() {return gr;}

[JoeC]

Re: Help With Copy Constructor.

Thanks for the time in explaining how that works.

That helped, ithink my graphic object is much better but it still
crashes at this point:

graphic::graphi c(const graphic& gr){
ud = lr = 16;
//gdata = gr.gdata; <-- This crashes
BITMAP bitmap = {0,ud,lr,2,1,1} ;
bitmap.bmBits = &gdata[0];
hbitmap = CreateBitmapInd irect(&bitmap);
}


That bity of code crashes when I use the command like this:

if(play){
cgr = new graphic(play->gOut());
return *cgr;
--------------
graphic gOut() {return gr;}

It works if I coment out that part but I get a block instead of my
graphic, I think I am getting close.

[Phlip]

Re: Help With Copy Constructor.

JoeC wrote:
[color=blue]
> graphic::graphi c(const graphic& gr){
> ud = lr = 16;[/color]

assert(this != & gr);
[color=blue]
> //gdata = gr.gdata; <-- This crashes[/color]

Are you sure you are not assigning to yourself?

--
Phlip
http://c2.com/cgi/wiki?ZeekLand <-- NOT a blog!!!

[Dennis Jones]

Re: Help With Copy Constructor.


"JoeC" <enki034@yahoo. com> wrote in message
news:1147308664 .035244.16280@i 40g2000cwc.goog legroups.com...[color=blue]
> Thanks for the time in explaining how that works.
>
> That helped, ithink my graphic object is much better but it still
> crashes at this point:
>
> graphic::graphi c(const graphic& gr){
> ud = lr = 16;
> //gdata = gr.gdata; <-- This crashes
> BITMAP bitmap = {0,ud,lr,2,1,1} ;
> bitmap.bmBits = &gdata[0];
> hbitmap = CreateBitmapInd irect(&bitmap);
> }
>
>
> That bity of code crashes when I use the command like this:
>
> if(play){
> cgr = new graphic(play->gOut());
> return *cgr;
> --------------
> graphic gOut() {return gr;}
>
> It works if I coment out that part but I get a block instead of my
> graphic, I think I am getting close.[/color]

Is 'gdata' a vector?

- Dennis

[Dennis Jones]

Re: Help With Copy Constructor.


"Phlip" <phlipcpp@yahoo .com> wrote in message
news:uxw8g.6899 3$_S7.7830@news svr14.news.prod igy.com...[color=blue]
> JoeC wrote:
>[color=green]
> > graphic::graphi c(const graphic& gr){
> > ud = lr = 16;[/color]
>
> assert(this != & gr);
>[color=green]
> > //gdata = gr.gdata; <-- This crashes[/color]
>
> Are you sure you are not assigning to yourself?[/color]

I think you're onto something there, Phlip. I hadn't even considered that
possibility. Joe, if the 'assert' fails, then that's definitely your
problem. However, rather than asserting, it would be better to handle the
special case:

graphic::graphi c(const graphic& gr)
{
if ( this == &gr ) return;
...
}

- Dennis

[JoeC]

Re: Help With Copy Constructor.

It is now I made the changes that were suggested. It does work better
if I take that statment out it just displays a block and don't copy the
graphic info. If I try to copy the program crashes.

[Dennis Jones]

Re: Help With Copy Constructor.


"JoeC" <enki034@yahoo. com> wrote in message
news:1147390630 .168736.142360@ u72g2000cwu.goo glegroups.com.. .[color=blue]
> It is now I made the changes that were suggested. It does work better
> if I take that statment out it just displays a block and don't copy the
> graphic info. If I try to copy the program crashes.[/color]

Again, you'll need to provide more info. -- just saying "the program
crashes" isn't very helpful. Have you tried stepping through the code with
a debugger to watch the values of critical variables when it crashes? Maybe
you should ask in one of the Windows newsgroups -- this group is about C++
in general, not Windows.

- Dennis

[JoeC]

Re: Help With Copy Constructor.

Again thanks for the help. I thought this was more of a c++ problem.
If gdata = gr.gdata; copys the vector then it should work. I see no
reason why id dosn't. I don't need to copy each element or dop I need
to have a getter statment from gr object in the copy constructor.

[Dennis Jones]

Re: Help With Copy Constructor.


"JoeC" <enki034@yahoo. com> wrote in message
news:1147450057 .472564.16520@i 40g2000cwc.goog legroups.com...[color=blue]
> Again thanks for the help. I thought this was more of a c++ problem.
> If gdata = gr.gdata; copys the vector then it should work. I see no
> reason why id dosn't.[/color]

Exactly -- from a C++ perspective, it should work. If it doesn't, there is
something else wrong, perhaps in your use of Windows code.

[color=blue]
> I don't need to copy each element[/color]

I'm not exactly sure what you are saying here. If you are referring to the
vector elements, then no, you do not need to copy each element of the vector
because a single assignment copies the entire contents of the vector into
the new vector. However, if you are referring to the class members, then I
think it makes good sense to copy each of the members, even if you could
initialize them explicitly -- I think it makes the intent of the copy
constructor clearer (you're copying the contents of one object into another)
and it will probably lead to better C++ habits in the long run.

[color=blue]
> or dop I need
> to have a getter statment from gr object in the copy constructor.[/color]

Again, I'm not sure what you are asking. You do not need a getter method to
access data members of the same class -- private members are always
accessible directly.

- Dennis

[JoeC]

Re: Help With Copy Constructor.

If it should work, then I might have been OK on the C++ side. I have
learned quite a bit with project. I have read in theory about objects
and copying but it is another thing to do it in practice. I will ask
on another group.