I recently got the urge (as you do) to convert a 1980s BASIC program to C++, first as a console program and then as a Windows one. Converting it to a console program was not too bad but the Windows version, though working, looks very messy and I was wondering if anyone had any ideas as to how to make it look neater. The problems are with display, keyboard input and delays, and I'll give an example of the latter here.

In the console version, part of it goes as follows:

switch (mm) {
case Bash:
if (u == Duck) {
printf("Your attempted dodge - ");
sleep(3);
if (random(20) < d) { printf("success!\n"); break; }
printf("No luck.\n"); }
printf("\nHits on you are ");
g = ((double)t / r) + 0.999; printf("%d\t", g); c -= g;
sound();
break;

The program is a "Gladiators" game. If your opponent decides to "bash" you will normally suffer damage, unless your move (chosen without seeing his) is a duck. In which case you may avoid the blow, it depends on chance and on your dexterity. There is an exciting delay while you find out.

But a Windows program is not supposed to sit around doing nothing. So this part now looks like:

switch (mm) {
case Bash:
if (u == Duck) {
m1 = _TEXT("Your attempted dodge - ");
sleep(3, wait1);
return;
Wait1:
if (random(20) < d) { m2 = _TEXT("success!"); break; }
m2 = _TEXT("No luck."); }
g1 = ((double)t / r) + 0.999; c -= g1;
break;

To make it work, I have a "sleep" function which actually sets a counter:

void sleep(int count, states act) {
counter = count;
state = act;
refresh();
}

I start a timer and handle it as follows:

case WM_TIMER:
if (state >= wait1 && state <= wait4 && counter > 0) {
counter--;
if (counter == 0) glad(state, 0); }
break;

and at the start of the main routine "glad" I have:

void glad(states act, TCHAR key) {
if (act == wait1) goto Wait1;

I'm sure that a goto jumping into an if in a switch is not anybody's idea of nice code. It also only works because most of the variables are globals rather than autos.

So is there a nicer way of doing it, while still making it fairly clear what is going on?

On Tuesday, 20 June 2023 at 12:03:47 UTC+1, Paul N wrote:
>
> To make it work, I have a "sleep" function which actually sets a counter:
>
> void sleep(int count, states act) {
> counter = count;
> state = act;
> refresh();
> }
>
> I start a timer and handle it as follows:
>
> case WM_TIMER:
> if (state >= wait1 && state <= wait4 && counter > 0) {
> counter--;
> if (counter == 0) glad(state, 0); }
> break;
>
> and at the start of the main routine "glad" I have:
>
> void glad(states act, TCHAR key) {
> if (act == wait1) goto Wait1;
>
> I'm sure that a goto jumping into an if in a switch is not anybody's idea of nice code. It also only works because most of the variables are globals rather than autos.
>
> So is there a nicer way of doing it, while still making it fairly clear what is going on?
>
You make the program event-driven. The events will include window open and close, but most importantly
a WM_TIMER call. You arrange for a periodic WM_TIMER call every tenth of. a second or so. The response is to
call a function called tick().

tick() calls daemons.

You need some way of getting the current time and comparing two times. The standard library
functions are not very suitable and Windows has a file time stamp function you can use for the
purpose.

typedef struct
{ TIMESTAMP triggertime;
void (*function)(void *ptr);
void *ptr;
} DAEMON;

DEAMON *g_daemons = 0;
int g_Ndaemons = 0;
int g_daemonbuffsize = 0;

/* add a daemon to the list */
void setdaemon(TIMESTAMP *t, void (*fptr)(void *), void *ptr)

/* tick calls the daemons and removes them from the list */
void tick(void)

Since as designed the program has no way of recovering if the daemon
list become empty, you probably want a special function that is always
called as well.

Don't use C function pointers in C++, use function<>-objects.

On Tuesday, June 20, 2023 at 5:25:55 PM UTC+1, Malcolm McLean wrote:
> On Tuesday, 20 June 2023 at 12:03:47 UTC+1, Paul N wrote:
> >
> > To make it work, I have a "sleep" function which actually sets a counter:
> >
> > void sleep(int count, states act) {
> > counter = count;
> > state = act;
> > refresh();
> > }
> >
> > I start a timer and handle it as follows:
> >
> > case WM_TIMER:
> > if (state >= wait1 && state <= wait4 && counter > 0) {
> > counter--;
> > if (counter == 0) glad(state, 0); }
> > break;
> >
> > and at the start of the main routine "glad" I have:
> >
> > void glad(states act, TCHAR key) {
> > if (act == wait1) goto Wait1;
> >
> > I'm sure that a goto jumping into an if in a switch is not anybody's idea of nice code. It also only works because most of the variables are globals rather than autos.
> >
> > So is there a nicer way of doing it, while still making it fairly clear what is going on?
> >
> You make the program event-driven. The events will include window open and close, but most importantly
> a WM_TIMER call. You arrange for a periodic WM_TIMER call every tenth of. a second or so. The response is to
> call a function called tick().

When I was mentally drafting this post I was going to include the phase "event-driven", but it seems not to have made it to the final version. Yes, my problems stem from the fact that the program is not supposed to sit around doing nothing when it pauses; likewise it is not supposed to actually wait for a key to be pressed. In each case it is supposed to "stop", and then be prodded back to life by an incoming message.

> tick() calls daemons.
>
> You need some way of getting the current time and comparing two times. The standard library
> functions are not very suitable and Windows has a file time stamp function you can use for the
> purpose.
>
> typedef struct
> {
> TIMESTAMP triggertime;
> void (*function)(void *ptr);
> void *ptr;
> } DAEMON;
>
> DEAMON *g_daemons = 0;
> int g_Ndaemons = 0;
> int g_daemonbuffsize = 0;
>
> /* add a daemon to the list */
> void setdaemon(TIMESTAMP *t, void (*fptr)(void *), void *ptr)
>
> /* tick calls the daemons and removes them from the list */
> void tick(void)
>
> Since as designed the program has no way of recovering if the daemon
> list become empty, you probably want a special function that is always
> called as well.

Thanks Malcom, but I think you mis-understand what I'm getting at. I currently have a timer which "ticks" every second. Normally "counter" is zero. When I want something to happen in the future I set "counter" to be the number of seconds delay and "state" to indicate what should happen. At each tick, counter is decreased if it is positive, and something happens if this reduces counter to zero. The length of the delay may vary by up to a second or so depending on exactly when it starts relative to the tick, but I am happy with this and I don't think I need to worry about actual timestamps.

What I am asking about is the ugliness (in my view) of the code that gets executed when the event (ie the end of the delay) happens, it jumps into an "if" which is itself in a "switch". Perhaps the code could be tidied if I included a delay whether or not the player is attempting to avoid a blow so at least the code paths are a bit more consistent.

On Tuesday, June 20, 2023 at 8:03:56 PM UTC+1, Bonita Montero wrote:
> Don't use C function pointers in C++, use function<>-objects.

Can you give an example? There are no function pointers in the code as written, but there is an enum which indicates to which label a jump should be made.

On 6/20/2023 2:52 PM, Paul N wrote:
> On Tuesday, June 20, 2023 at 8:03:56 PM UTC+1, Bonita Montero wrote:
>> Don't use C function pointers in C++, use function<>-objects.
>
> Can you give an example? There are no function pointers in the code as written, but there is an enum which indicates to which label a jump should be made.

Perhaps Bonita is referring to:

typedef struct
{ TIMESTAMP triggertime;
void (*function)(void *ptr);
void *ptr;
} DAEMON;

Written my Malcolm? Bonita omitted it in the response... ;^o

Am 20.06.2023 um 23:52 schrieb Paul N:
> On Tuesday, June 20, 2023 at 8:03:56 PM UTC+1, Bonita Montero wrote:
>> Don't use C function pointers in C++, use function<>-objects.
>
> Can you give an example? There are no function pointers in the code as written, but there is an enum which indicates to which label a jump should be made.

That's an example:

void fileList( wchar_t const *path, function<bool ( WIN32_FIND_DATAW
const & )> const &callback )
{ WIN32_FIND_DATAW fd;
HANDLE hFind = FindFirstFileW( path, &fd );
int ec;
auto thr = [&]() { throw system_error( ec, system_category(), "error
with FindFirstFileW()" ); };
if( hFind == INVALID_HANDLE_VALUE )
if( (ec = GetLastError()) != ERROR_FILE_NOT_FOUND )
thr();
else
return;
for( ; ; )
{
if( !callback( fd ) )
goto bye;
if( !FindNextFileW( hFind, &fd ) )
if( (ec = GetLastError()) != ERROR_FILE_NOT_FOUND )
thr();
else
goto bye;
}
bye:
FindClose( hFind );
}

You can pass a lambda to fileList and it is automatically converted. If
you use a templated function object you'd have multiple instantiations
of fileList. With the above code you won't need to put the above code
into a header.

More complete:

#include <Windows.h>
#include <iostream>
#include <functional>
#include <system_error>

using namespace std;

void fileList( wchar_t const *path, function<bool ( WIN32_FIND_DATAW
const & )> const &callback, char const *errWhat );

int main()
{ fileList( L"*", []( WIN32_FIND_DATAW const &fdw ) -> bool
{
wcout << fdw.cFileName << endl;
return true;
}, "error listing files" );
}

void fileList( wchar_t const *path, function<bool ( WIN32_FIND_DATAW
const & )> const &callback, char const *errWhat )
{ WIN32_FIND_DATAW fd;
HANDLE hFind = FindFirstFileW( path, &fd );
int ec;
auto thr = [&]() { throw system_error( ec, system_category(), errWhat ?
errWhat : "" ); };
if( hFind == INVALID_HANDLE_VALUE )
if( (ec = GetLastError()) != ERROR_FILE_NOT_FOUND )
thr();
else
return;
for( ; ; )
{
if( !callback( fd ) )
return (void)FindClose( hFind );
if( !FindNextFileW( hFind, &fd ) )
if( (ec = GetLastError()) != ERROR_NO_MORE_FILES )
thr();
else
return (void)FindClose( hFind );
}
}

On Tuesday, June 20, 2023 at 12:03:47 PM UTC+1, Paul N wrote:
> But a Windows program is not supposed to sit around doing nothing. So this part now looks like:
>
> switch (mm) {
> case Bash:
> if (u == Duck) {
> m1 = _TEXT("Your attempted dodge - ");
> sleep(3, wait1);
> return;
> Wait1:
> if (random(20) < d) { m2 = _TEXT("success!"); break; }
> m2 = _TEXT("No luck."); }
> g1 = ((double)t / r) + 0.999; c -= g1;
> break;

I've managed to improve it a bit, as follows:

Wait1:
switch (mm) {
case Bash:
if (u == Duck) {
if (act != wait1) {
m1 = _TEXT("Your attempted dodge - ");
sleep(3, wait1);
return; }
if (random(20) < d) { m2 = _TEXT("success!"); break; }
m2 = _TEXT("No luck."); }
g1 = ((double)t / r) + 0.999; c -= g1;
break;

This version relies on mm and u not changing between the two invocations, so the Wait1: label is not now buried in an "if" or a "switch". Indeed, I may be able to use an outer "switch" to get rid of the "goto" altogether.

Any more improvements than anyone can see?