Tech Support > Microsoft Windows > Development Resources > Question about my program
Question about my program
Posted by JoeC on February 15th, 2006


I am trying to create a simple maze program on a window. I am trying
to create a global var to handle the commands.

I declare:

#include "space.h"
#include "player.h"
#include "Command.h"

static char gCmd = '$'; <--- My global command.

#ifndef DUNFUNCS_H
#define DUNFUNCS_H

int roll(int, int);
LRESULT CALLBACK WinProc(HWND, UINT, WPARAM, LPARAM);
void DrawScreen(HDC);
void ScreenSetup(HWND);
int setup(space (&b)[30][30], player*, const int);
ifstream& MapIn(ifstream&, char&);
void fill(space (&spaces)[30][30], const int);
void loop(HWND, space (&spaces)[30][30], const int);


#endif

In My WinProc:

case WM_PAINT:
HDC hdc;
PAINTSTRUCT ps;
hdc = BeginPaint(hwnd, &ps);
DrawScreen(hdc);
EndPaint(hwnd, &ps);
return 0;

case WM_COMMAND:
if(LOWORD(wparam)== 1){
SendMessage(GetParent((HWND) lparam), WM_DESTROY, 0, 0);
}
if(LOWORD(wparam)==2){
Command = 'n';
gCmd = Command; <--Assigns the Command to global

Command Cmd(gCmd);<--Puts the Global var in the command object

Here in my loop it seems that the var dosn't take the value of 'n'
when I display gCmd it is still '$'

How can I fix this, I think it is a problem of scope and static
variables.

Posted by Alf P. Steinbach on February 15th, 2006


* JoeC:
Presumably the above #include defines some command-related things.

You should probably have included or described that.

We're not telepathic.


In general it's not a good idea to use globals.

On the other hand, for the kind of program you're doing (beginner's
Windows API) it does make sense and is The Right Thing To Do.

On the third and most important gripping hand, you're making the wrong
thing into a global. A command is not part of the window state, it's an
action. Before using globals in a good way for novice windowing
program, you must master how to /not/ use globals.


If this is a header file you should not define variables in it anyway.


You'll avoid a lot of problems if you simply refrain from
forward-declaring things.

Anyway, don't put internal implementation details in a header file.

That will also help you avoid a lot of problems.



It's a good idea to put such code in a separate function.


Here you have at least one bug, and possibly two. Instead of analyzing
low-level API data at the bits & bytes level, use the relevant API
facilities (in this case, the macros from <windowsx.h>). And know your
APIs! I won't discuss the details here since this group is about C++,
but because you don't know your APIs, you're using the
(Windows-specific) message WM_DESTROY incorrectly. Put this question up
for discussion in e.g. [comp.os.ms-windows.programmer.win32].


Again, don't analyze data at the bits & bytes level.


Is Command a type or a variable, and where the heck did it come from?


D O N ' T D O T H A T !

The main problem is you're trying to tackle Windows API programming
while you're still struggling with the most fundamental programming
concepts (like, what's a variable and what's a type and so on, not to
mention the divide between states and actions).

Best advice: do something _very much_ simpler.


--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Posted by Matt on February 15th, 2006


JoeC wrote:
You shouldn't define static variables in a header file. A static
variable at file scope creates the storage for that variable and
it also confines the visibility of that variable to the file that
it's defined in. So if, say, you put that "static char gCmd;"
in "command.h" and include that in "main.c" "command.c" and
"maze.c", then you'll wind up with three variables called gCmd,
one in main.c, one in command.c and one in maze.c. and none of
them can be accessed or referenced outside of those modules.

What you probably want to do is define one non-static gCmd
variable to allocate the storage for it, "char gCmd;" and then
use "extern char gCmd;" in the files that use that variable.
"Extern" doesn't create storage, it just tells the compiler
about gCmd so the compiler can do type checking and so forth.
You can put extern variable declarations in a header if you
want.

Incidentally, I agree with Alf about avoiding globals, but
I think you should learn the syntax first. You can worry
about designing things once you learn the language.

Matt

Posted by JoeC on February 16th, 2006


Thanks for the advice. There are pleanty of resources on how to C++
code but I can't find much on how to design a program. I havn't had
any formal progamming training in highschool about twenty years ago. I
want to learn programming and the only way to do it is just do it.

I do try to avoid golbal variables because I know they are bad but I am
not sure how to have the WinProc loop control the program. I have to
get the actions chosen in that function to the main program loop.
Finally the > if(LOWORD(wparam)==2){ stuff is straight out
of a book on how to do the COMMAD:...

Posted by JoeC on February 16th, 2006


Thanks for the advice. There are pleanty of resources on how to C++
code but I can't find much on how to design a program. I havn't had
any formal progamming training since I was in highschool about twenty
years ago. I want to learn programming and the only way to do it is
just do it.

I do try to avoid golbal variables because I know they are bad but I am
not sure how to have the WinProc loop control the program. I have to
get the actions chosen in that function to the main program loop.
Finally the > if(LOWORD(wparam)==2){ stuff is straight out
of a book on how to do the COMMAD:...

Posted by JoeC on February 16th, 2006


Oh, thank you for that explination, it all makes sence now. Thanks
much I will give it a try.

Posted by JoeC on February 16th, 2006


Thanks so much you helped me greatly with that aspect of my design.
There are a great deal of books about code but not many that I have
seen about design. I am writing a rather complex program. I am trying
to make maze program in a window. The only way to get experience is to
just do try and ask for help. I still have pleanty of other bugs, I
will work on them but if I am stuck I will ask.

Posted by Alf P. Steinbach on February 16th, 2006


* JoeC:
Yes, that's a good way -- and perhaps the only way that works.


I just realized that you've /crossposted/ the thread to [comp.lang.c++]
and [comp.os.ms-windows.programmer.win32].

Please don't do that, since Windows-specific stuff is completely
off-topic in [comp.lang.c++].

I'll respond to the last Windows-specific paragraph, which I've snipped
here, in [comp.os.ms-windows.programmer.win32] only.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Posted by Alf P. Steinbach on February 16th, 2006


* JoeC:
Huh?


The [windowsx.h] include file contains a large number of macros that
unpack and pack message arguments.

E.g., where you currently have

case WM_COMMAND:
if(LOWORD(wparam)== 1){
SendMessage(GetParent((HWND) lparam), WM_DESTROY, 0, 0);
}
... etc.

you should have

case WM_COMMAND: return HANDLE_WM_COMMAND( messageId, wParam,
lParam, onWmCommand );

where messageId is the message id argument to your window proc (you
didn't show that part of the code), and onWmCommand is a function of the
form

void Cls_OnCommand(HWND hwnd, int id, HWND hwndCtl, UINT codeNotify)

which you find in a comment in [windowsx.h], on the line before the
definition of HANDLE_WM_COMMAND.

In that function, put all your current WM_COMMAND handling, using the
arguments passed to the function instead of LOWORD bits & bytes stuff.

On a totally unrelated note, I'm going to kill Thunderbird. It's got
bird flu, it seems. Screws up editing of quoted text and modifies what
I've written before sending it, i.e. not WYSIWYG even for plain text!

The only consolation is that Outlook Express is even worse.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Posted by Lucian Wischik on February 16th, 2006


"Alf P. Steinbach" <alfps@start.no> wrote:
I wouldn't go that far... the code he wrote is what you get directly
from reading the msdn docs, and from reading windows.h/winuser.h, and
doesn't require anything more. Whereas the crackers involve an extra
set of functions/macros to learn for negligable benefit.

--
Lucian

Posted by Alf P. Steinbach on February 16th, 2006


* Lucian Wischik:
On the contrary, using the message cracker macros you (1) do not have to
learn the packing, i.e. there's much _less_ one must learn and memorize,
and (2) will reduce the frequency of typo-style bugs, and (3) will
reduce the complexity of the code, by dividing it up in functions.

Of course the benefits can be the other way around for one who just
copies code literally (e.g. from books) without understanding anything.

But I wouldn't recommend the copy-and-modify-without-understanding way
of developing software, especially for the purpose of learning.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Posted by Matt on February 16th, 2006


Alf P. Steinbach wrote:
I find it to be a huge benefit. I've extended it quite a bit and I
use that file for pretty much every standard GUI thing. Another
thing is that you can use Ctags with functions, which you can't do
with case statements. You can also inline all the message handler
functions if you're concerned about runtime overhead. The biggest
bonus about it though is that you have all the function prototypes
right there in one file. I mapped up a key combination in Vim
that will call a script to scan through that file and insert
message handler functions for me. This setup makes it totally
effortless to deal with messages. No MSDN required, since you have
the function parameters right there documenting the message. Plus
if there's any weird stuff like those scroll bar switch statements
you always need, you can just throw it in the script. I highly
recommend it.

Matt

Posted by JoeC on February 16th, 2006


This part works as I intend it. I guess there are better ways but I
got this from nitty gritty windows programming. From the advice I got
this part of my program works. I am still working on other parts. I
know they are buggy but I am trying to come up with better ways of
writing these parts.

#include<windows.h>
#include<stdlib.h>

#include "dunfuncs.h"
#include "Command.h"

void loop(HWND hwnd, space (&spaces)[30][30], const int num){

extern char gCmd;
int l1;
int l2;
Command Cmd(gCmd);
coord newLoc;
coord Tloc;

coord loc;
for(l1=0; l1 != num; l1++){
for(l2=0; l2 != num; l2++){
if(spaces[l1][l2].isPlay()){
loc.x = l1;
loc.y = l2;
}
}
}

Tloc.x = loc.x;
Tloc.y = loc.y;

for(l1=(loc.x-1); l1 <= (loc.x+1); l1++){
for(l2=(loc.y-1); l2 <= (loc.y+1); l2++){
spaces[l1][l2].see();
}
}

char gr;
char * pgr;
HDC hdc;
PAINTSTRUCT ps;
hdc = BeginPaint(hwnd, &ps);

for(l1=0; l1 < num; l1++){
for(l2=0; l2 != num; l2++){
gr = spaces[l1][l2].graphicOut();
pgr = &gr;
if(spaces[l1][l2].been()){
TextOut(hdc, l1*15,l2*15,pgr,1);
pgr=0;
}

}
}

char gr2;
char * pgr2;
gr2 = Cmd.rCmd();
pgr2 = &gCmd;
TextOut(hdc,400,400,pgr2,1);
EndPaint(hwnd, &ps);


newLoc = Cmd.CmdOut();
if(newLoc.x != 0 || newLoc.y != 0){
MessageBox(NULL, "You are Trying to Walk Through a Wall!" , "Dumb
Ass!", MB_OK);}
Tloc.x = Tloc.x + newLoc.x;
Tloc.y = Tloc.y + newLoc.y;

char val[4];
itoa(newLoc.x, val, 10);

if (newLoc.x != 0){
MessageBox(NULL, val , "Dumb Ass!", MB_OK);}


if(!spaces[Tloc.x][Tloc.y].canMove()){
MessageBox(NULL, "You are Trying to Walk Through a Wall!" ,
"Error!", MB_OK);
return;}

player * pl = spaces[loc.x][loc.y].PlayOut();
spaces[Tloc.x][Tloc.y].playIn(pl);


}

#include<map>

#include "coord.h"

#ifndef COMMAND_H
#define COMMAND_H

class Command{
char mCmd;
std::map<char, coord>mover;
coord n;
coord s;
coord e;
coord w;

public:
Command();
Command(char c) : mCmd(c){ }
coord CmdIn(char);
coord CmdOut(){return mover[mCmd];}
char rCmd(){return mCmd;}
//void CmdIn(char);
};

#endif

#include<map>
#include "Command.h"

Command::Command(){

n.x=-1;
n.y=0;
s.x=1;
s.y=0;
e.x=0;
e.y=1;
w.x=0;
w.y=-1;

mover['n']=n;
mover['s']=s;
mover['e']=e;
mover['w']=w;
mCmd=' ';
}

coord Command::CmdIn(char cmd){

mCmd = cmd;
mCmd = ' ';
return mover[cmd];


}


#include<map>
#include "Command.h"

Command::Command(){

n.x=-1;
n.y=0;
s.x=1;
s.y=0;
e.x=0;
e.y=1;
w.x=0;
w.y=-1;

mover['n']=n;
mover['s']=s;
mover['e']=e;
mover['w']=w;
mCmd=' ';
}

coord Command::CmdIn(char cmd){

mCmd = cmd;
mCmd = ' ';
return mover[cmd];


}


#include "space.h"
#include "player.h"

space::space(){
play = 0;
graphic = ' ';
seen = false;
}

char space::graphicOut(){

if(play){return play->gOut();}
if(seen){return graphic;}
}

void space::commandIn(char cmd){

}

bool space::isPlay(){
if(play){return true;}
return false;
}

player* space::PlayOut(){
player * p = play;
play = 0;
return p;
}

bool space::canMove(){
if(graphic != '#'){
return true;}
else {return false;}
}

Posted by Lucian Wischik on February 16th, 2006


"JoeC" <enki034@yahoo.com> wrote:
Sorry to be so blunt, but your entire program is badly structured!

In essence, you should no longer use your own loop. Instead you should
store your state in some data structure, and then respond to events.
One event will be WM_PAINT, in response to which you repaint your
window. Another event will be a keypress, in response to which you
change the state in your appropriate way.

So: your WinMain does RegisterClass, then allocates (using "new") an
instance of your state-structure, then does CreateWindow() passing a
pointer to this instance, then enters its message-loop (read MSDN
about this), then quits.

Your WndProc for your window retreives the pointer that was passed in
CreateWindow and stores it using SetProp(...) or SetWindowLongPtr().
Every subsequent message-handler retreives the state using GetProc or
GetWindowLongPtr(...). In this way you no longer need global
variables.

In response to WM_PAINT, you get the pointer to your instance data, do
BeginPaint, paint the appropriate things, then EndPaint.

In response to WM_KEYDOWN, you get the pointer to your instance data,
update it as appropriate in response to the key, and call
InvalidateRect() so that your app knows it needs to repaint itself.


You need to think carefully about your state, and about how your state
gets transformed by the various messages. You should write it down on
a piece of paper. Write a list of what the state consists of
(presumably the map, the current coordinates, and a mask to say which
squares of the map have been explored so far).

In response to the up-arrow keypress, you check whether there's a wall
to the north, pop up a messagebox if there is; and if there isn't,
then you update the current state (i.e. coordinates and visible-mask)
and call InvalidateRect.

--
Lucian

Posted by JoeC on February 17th, 2006


Sorry to be so blunt, but your entire program is badly structured!

No problem. That is a result of trying to get it to work. I am taking
the basic format that I used for a DOS window program that worked
pretty well. I should put most of the action in the WinProc loop and
have most of the action in there.


Similar Posts