Home » Category » Microsoft Visual C & C++

Microsoft Visual C & C++: Clist problem

202| Wed, 06 Feb 2008 10:15:00 GMT| thomasgreenwood| Comments (8)
I have written a program that draws ellipses containing a label on the screen where the user clicks the left mouse button, I have stored the data for each ellipse (its rect stuff and the label) and when I click on the ellipse it should print a msgbox telling me the label of the ellipse, however, the labels all seem to be the same as the one at the head of the list, I have also checked the list and found the labels in all the list elements to be that of the head element, here is the main piece of code I am working on:

class CRegState
{
public:
char* label;
RECT state;

};
// ************* LIST DECLARATION *****************************

CList<CRegState,CRegState&> statelist;

// ************************************************************

void CRegister::OnLButtonDown(UINT nFlags, CPoint point)
{

CRect rect;
CRegState thisstate;
char* slabel;

// check Ok button has been clicked
if (onclicked == TRUE)
{


int mouseX=(point.x-20);
int mouseY=(point.y-20);

CClientDC dc(this); // Device context for painting etc

//rect.Ellipse(mouseX,mouseY,mouseX+50,mouseY+50);

dc.Ellipse(mouseX,mouseY,mouseX+50,mouseY+50);
rect = CRect(mouseX,mouseY,mouseX+50,mouseY+50);
dc.TextOut(mouseX+20,mouseY+16,str);
onclicked = FALSE;
thisstate.label = str;
thisstate.state = rect;
statelist.AddHead(thisstate);



}
// test to see if in ellipse
POSITION pos = statelist.GetHeadPosition( );

while (pos)
{
//thisstate = statelist.GetHead();
thisstate = statelist.GetNext(pos);
if(PtInRect(&thisstate.state,point)) {
slabel = thisstate.label;
MessageBox(slabel,"You're inside!",MB_OK);
}
}
}

can anyone shed any light on why this is happening?

thanks

Tom.

Keywords & Tags: clist, microsoft, visual c++, vc

URL: http://www.7prog.com/visual-c-c++/52462/
 
«« Prev - Next »» 8 helpful answers below.
hi there,

glancing through this piece of code, i see a potential problem point:
it seems you're adding the adres of 'CRegState thisstate;' to your list.
The bad thing is that the variable 'thisstate' exists only inside the 'OnLButtonDown' function, but not anymore outside.
Each time you enter this function, the 'thisstate' is re-created. In your case, it is recreated at the same memory adress, thereby overwriting its previous value.

The 'statelist' consists of a lot of pointers to the same adres: 'CRegState thisstate' in memory, which, when you're not in the function 'OnLButtonDown' isnt even a valid memory-adres!!
You are lucky you didnt get exception-errors!

A possible solution might be to declare CRegState* thisstate; //undefined pointer to a CRegState class

when you get into 'adding' routine
if (onclicked == TRUE)
{
thisstate = new CRegState; //create a new CRegState instance
...

fill it up with your values: thisstate->label = str; // .. and so on

Modify the 'statelist' to take CRegState*, modify all 'thisstate' accesspoints (i.e. from 'thisstate.' to 'thisstate->') and dont forget to delete all 'CRegState's when you're done with them.

Good luck!

mihalidis | Sat, 10 Nov 2007 05:10:00 GMT |

thanks for, you help, I have modified the code to this:

class CRegState
{
public:
char* label;
RECT state;

};
// ************* LIST DECLARATION *****************************

CList<CRegState,CRegState&> statelist;

// ************************************************************

void CRegister::OnLButtonDown(UINT nFlags, CPoint point)
{

CRect rect;
CRegState *thisstate;
char* slabel;


// check Ok button has been clicked
if (onclicked == TRUE)
{

thisstate = new CRegState; //create a new CRegState instance
int mouseX=(point.x-20);
int mouseY=(point.y-20);

CClientDC dc(this); // Device context for painting etc

//rect.Ellipse(mouseX,mouseY,mouseX+50,mouseY+50);

dc.Ellipse(mouseX,mouseY,mouseX+50,mouseY+50);
rect = CRect(mouseX,mouseY,mouseX+50,mouseY+50);
dc.TextOut(mouseX+20,mouseY+16,str);
onclicked = FALSE;
thisstate->label = str;
thisstate->state = rect;
statelist.AddHead(*thisstate);

}

// test to see if in ellipse
POSITION pos = statelist.GetHeadPosition( );

while (pos)
{

*thisstate = statelist.GetNext(pos);
if(PtInRect(thisstate.state,point)) {
slabel = thisstate.label;
MessageBox(slabel,"You're inside!",MB_OK);
}
}
}

but I get the following errors:

:\My Documents\STAR\Register.cpp(145) : error C2228: left of '.state' must have class/struct/union type
Z:\My Documents\STAR\Register.cpp(146) : error C2228: left of '.label' must have class/struct/union type
Error executing cl.exe.

and I don't understand how to correct them, also I have compiled the program without the code checking if I'm in the ellipse and it still seems to be setting all the elements in the list to that of the head, can you tell me where I am going wrong? Once again thanks for your time and help.

thomasgreenwood | Sat, 10 Nov 2007 05:11:00 GMT |

and something else which slipped my attention is the following: its nescesary to allocate space for the CRegState.label, because its a pointer to a 'char', but it is not initialized yet, i.e. pointing to some random memory adress.
So you need to do a:

CString str; // is this how you declared it?
dc.TextOut(mouseX+20,mouseY+16,str);
...
thisstate->label = new char[str.GetLength()+1]; //declare enough space even for the null-terminator.
sprintf(thisstate->label, "%s", (LPCTSTR)str); //fill it up.

and ofcourse you need to deallocate this space too.

..Sorry i didnt catch this the first time

mihalidis | Sat, 10 Nov 2007 05:12:00 GMT |

Another possible solution is to Have CList hold the object instead of a pointer/reference.
CList <CRegState,CRegState> myList;

You will need to overload the operator= and write a copy constructor for your CRegState Class.
The nice part about doing it this way is all the memory allocation is done for you. Albeit a list of pointers is faster because there isn't all that copying going on.
Take Care,
Bob

bobcarleone | Sat, 10 Nov 2007 05:13:00 GMT |

hmm..

The error you're getting is a basic error. Not understanding it shows you should probably perk up your knowledge about the concept of pointers in C, and memory usage in general before you move on.

Its really tough to help you unless you get more familiair about these issues.

i suggest you read some books. Try a 'C++ in 21 days' type-of-book (there are plenty around), which can help you well on your way.

mihalidis | Sat, 10 Nov 2007 05:14:00 GMT |

The problem is I haven't written any C++ for over a year and I need to have this program finished in 5 weeks as part of my degree, so I'd really apprecaite the help. Also I didn't declare str as CString, I used char str[5]. because I am using GetWindowText to get text from an edit box and the first parameter is a pointer to a string, and with the new code you suggested this causes an error, how can I get round this- sorry for my lack of knowledge!

Tom

thomasgreenwood | Sat, 10 Nov 2007 05:15:00 GMT |

Thanks for your help, I've now got it working, in case you're interested the final version of my code is:

class CRegState
{
public:
char* label;
RECT state;

};
// ************* LIST DECLARATION *****************************

CList<CRegState,CRegState&> statelist;

// ************************************************************

void CRegister::OnLButtonDown(UINT nFlags, CPoint point)
{

CRect rect;
CRegState *thisstate;
char* slabel;


// check Ok button has been clicked
if (onclicked == TRUE)
{

thisstate = new CRegState; //create a new CRegState instance


int mouseX=(point.x-20);
int mouseY=(point.y-20);

CClientDC dc(this); // Device context for painting etc


dc.Ellipse(mouseX,mouseY,mouseX+50,mouseY+50);
rect = CRect(mouseX,mouseY,mouseX+50,mouseY+50);
dc.TextOut(mouseX+20,mouseY+16,str);
onclicked = FALSE;
thisstate->label = new char[str.GetLength()+1]; //declare enough space even for the null-terminator.
sprintf(thisstate->label, "%s", (LPCTSTR)str); //fill it up.
thisstate->state = rect;
statelist.AddHead(*thisstate);

}

// test to see if in ellipse
POSITION pos = statelist.GetHeadPosition( );

while (pos)
{

thisstate = &statelist.GetNext(pos);
if(PtInRect(&thisstate->state,point))
{
slabel = thisstate->label;
MessageBox(slabel,"You're inside!",MB_OK);
}
}
}

thanks again, Tom

thomasgreenwood | Sat, 10 Nov 2007 05:16:00 GMT |

Why are you doing this the hard (MFC) way?

#include <string>
class CRegState
{
public:
std::string label;
RECT state;
};
// ************* LIST DECLARATION *****************************
typedef std::list<CRegState> RegStateList;
RegStateList statelist;
//..
// ************************************************************
void CRegister::OnLButtonDown(UINT nFlags, CPoint point)
{
CRect rect;
CRegState thisstate;
std::string slabel;

// check Ok button has been clicked
if (onclicked == TRUE)
{

int mouseX=(point.x-20);
int mouseY=(point.y-20);

CClientDC dc(this); // Device context for painting etc

dc.Ellipse(mouseX,mouseY,mouseX+50,mouseY+50);
rect = CRect(mouseX,mouseY,mouseX+50,mouseY+50);
dc.TextOut(mouseX+20,mouseY+16,str);
onclicked = FALSE;
thisstate.label = str;
thisstate.state = rect;
statelist.push_back(thisstate);
}

// test to see if in ellipse
RegStateList::iterator it;
it = statelist.begin()
while (it != statelist.end())
{
thisstate = *it;
if(PtInRect(thisstate.state,point)) {
slabel = thisstate.label;
MessageBox(slabel,"You're inside!",MB_OK);
}
it++;
}
}

No new's, deletes, aches or pains. The trick is to use STL.

Regards,

Paul McKenzie

paulmckenzie | Sat, 10 Nov 2007 05:17:00 GMT |

Microsoft Visual C & C++ Hot Answers

Microsoft Visual C & C++ New questions

Microsoft Visual C & C++ Related Categories