Constructor question (again)

[JoeC]

I have been trying to get this copy constructor to work. I don't know
why it crashes.

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


I am using the function like this:

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


class player{

string name;
graphic gr;
void create();

public:
player();
graphic gOut(){return gr;}
void dummy(){MessageBox(NULL, "Dummy" , "Notice", MB_OK); }

};

 

[Heinz Ozwirk]

I have been trying to get this copy constructor to work. I don't know
why it crashes.

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

How is gdata defined and how does its assignment operator look like?

Heinz

 

[Jonathan Mcdougall]

I have been trying to get this copy constructor to work. I don't know
why it crashes.

We don't either. Make sure

1) it makes sense semantically to copy that object
2) you are using your library correctly
3) the rule of three is respected ("A class with any of {destructor,
assignment operator, copy constructor} generally needs all 3")
4) everything is fine *before* the line where it crashes (undefined
behavior)

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

Use initialization lists when you can:

graphic::graphic(const graphic& gr)
: ud(16), lr(16) ....

Since the code you provided is not sufficient to diagnose the error, I
cannot help you more. However, I suspect the problem comes from what
you are doing with Microsoft's GDI library. Try in a microsoft
newsgroup
(http://www.parashift.com/c++-faq-lite/how-to-post.html#faq-5.9).


Jonathan

 

[Victor Bazarov]

I have been trying to get this copy constructor to work. I don't know
why it crashes.

We don't know why it crashes either.

graphic::graphic(const graphic& gr){
ud = lr = 16;
gdata = gr.gdata; <-- This line crashes

It would seem that 'gr' is somehow invalid here. What does 'gdata'
contain? Is 'gr' OK here? How did you obtain this refernece?

BITMAP bitmap = {0,ud,lr,2,1,1};
bitmap.bmBits = &gdata[0];
hbitmap = CreateBitmapIndirect(&bitmap);
}


I am using the function like this:

if(play){
cgr = new graphic(play->gOut());

So, what's "play"? Is that pointer OK or is it also invalid?
Comparing it to zero is not necessarily enough to validate it. Who
creates it? Who fills (sets) it? Could it be that "play" has been
disposed of at some point before reaching this point?

You need to debug your program and make sure that when the program
gets to this point, 'play' is valid (points to a regular 'player'
object, still alive, with all fields still valid as well). We can't
do it for you.

return *cgr;
}


class player{

string name;
graphic gr;
void create();

public:
player();
graphic gOut(){return gr;}
void dummy(){MessageBox(NULL, "Dummy" , "Notice", MB_OK); }

};

V

 

[JoeC]

Sorry, here is the header for graphic:

class graphic{
int btmap;
int lr,ud; //Diminsion (size) of the graphic
std::vector<BYTE> gdata;
HBITMAP hbitmap;
HDC hdc, hdcmem;


public:
graphic();
graphic(const BYTE c[]);
graphic(const graphic&);
void set(const BYTE c[]);
BYTE getData(const int n)const {return gdata[0];}
void display(HWND,const int, const int);

};

 

[JoeC]

gdata is a vector. When I comment out the line the program works but I
get a block instead of my graphic. There seems to be a problem
transfering the graphic data in the vector.

 

[JoeC]

If there is no player then it will display a different graphic here is
the rest of the code:

graphic& space::graphicOut(){

if(play){
cgr = new graphic(play->gOut());
return *cgr;
}
if(seen){return *gr;}
else {return *grDefault;}
}
Don't worry, every thing else works. I have gotten this to work by
creating the graphic in this object. I did it like this:

graphic& space::graphicOut(){
//if(play){return play->gOut();} <-This is the way I want to do
the graphic
//but it crashes.
if(play){return *cgr;}
if(seen){return *gr;}
else {return *grDefault;}
}

 

[Jonathan Mcdougall]

gdata is a vector. When I comment out the line the program works but I
get a block instead of my graphic. There seems to be a problem
transfering the graphic data in the vector.

Please learn to quote correctly on Usenet. See
http://en.wikipedia.org/wiki/Top-posting#Inline_replies.

I reproduce and simplify your code here. You seem to think the problem
comes from the marked line:

graphic
{
public:
graphic(const graphic& gr)
{
gdata = gr.gdata; // <---- This line crashes
}

std::vector<BYTE> gdata;
};

If BYTE is a typedef for an unsigned char (or some builtin type), this
line has no reason to crash. It may throw an exception (for memory
allocation problems), but it cannot just "crash" (assuming the library
itself has no bug).

If BYTE is not a builtin type, then it may be its operator= or
something in that class that cause problems.

Having said that, I doubt that the statement itself causes a problem. I
suspect memory gets corrupted before that line. Again:

1) it makes sense semantically to copy that object
2) you are using your library correctly (GDI)
3) the rule of three is respected ("A class with any of {destructor,
assignment operator, copy constructor} generally needs all 3")
4) everything is fine *before* the line where it crashes (undefined
behavior)


Jonathan

 

[JoeC]

Here is more code:

class space{
char gchar;
graphic *gr;
graphic *grDefault;
graphic * cgr;
player * play;
bool seen;
void cleanup(){cgr = 0;}

public:
space();
~space();
void graphicIn(char g);
graphic& graphicOut();
void playIn(player*);
bool isPlay();
void see(){seen = true;}
bool been(){return seen;}
void playOut();
bool canMove();
bool winspace();
};


void space:layIn(player *p){
play = p;
seen = true;
}

 

[Victor Bazarov]

Here is more code:

[..]

Do you really believe that anybody here except you has enough time
to collect all the pieces you cared to post scattered across many
messages and then try to make sense of them? Either debug your code
(using all the suggestions already given) or rewrite it from scratch
(which sometimes seems easier than trying to find the mistake). Do
what many professionals do in similar situations: divide and conquer
your code. Figure out which part are truly working fine: make sure
that pre- and post-conditions are met by introducing assertions into
your code. Write unit tests and run them. IOW, begin developing
your program properly instead of trying to take it by storm.

V

 

[JoeC]

I know my problem is complex, and I do thank all those for your time.
actually did rewrite this program seveal times from scratch. This is
my best reslut so far. I thought I had a pretty good design for the
program but it is that one line that is realy confusing me. I can make
the program work as I would like but the reason why I am doing this way
is for expansion in the future. I never realy studdied programming and
there are gaps in my knowlege. Most of my learning comes from doing
projects like this. I have some ideas for trying to make this work the
way I want. I will put the project down for a while and try some of
the changes.

The concept behind my program is that I have a grid of space objects.
I want each space to be able potentially to hold a character but there
is only one character in the game and in one space.

If I do class space{
char ch;

I create a character in each space>

I decited to do char * ch;


Still I will do some research but for my problem I am not sure where to
look.

Again thanks for the help.

 

[benben]

If there is no player then it will display a different graphic here is
the rest of the code:

graphic& space::graphicOut(){

if(play){
cgr = new graphic(play->gOut());

The above line looks like leaking memory. Just a guess, I am not sure.

return *cgr;
}
if(seen){return *gr;}
else {return *grDefault;}
}
Don't worry, every thing else works. I have gotten this to work by
creating the graphic in this object. I did it like this:

graphic& space::graphicOut(){
//if(play){return play->gOut();} <-This is the way I want to do
the graphic
//but it crashes.
if(play){return *cgr;}
if(seen){return *gr;}
else {return *grDefault;}
}

If you really want us to help, you've gotta help us to help you. This
usually involves creating the minimal compilable program that reproduces
the problem.

Regards,
Ben