|
|
| How bad is a goto statement? |
|
Sirocco
Member #88
April 2000
|
I think the reasonable response is to say that GOTO is but one tool of many in a programmer's kit. A good programmer knows when and how often to implement the tools at his/her disposal; that is, it's there if you feel the situation warrants it. I've used a handful of GOTO statements in the past, and I'm sure at some point in the future it will crop up once more. I've larger things to worry about --> |
|
orz
Member #565
August 2000
|
I think that the anti-goto rhetoric is largely a leftover from the bygone era when many programers learned to program in asm or basic where gotos and jmps were natural. People who started with C or higher level languages usually use higher level flow control primitives because, for most code, they're easier to write, maintain, and think in. Quote: Most of Allegro's goto usage is for error handling. Given C's lack of exceptions, there's not always a good alternative.
That's what longjmp() is for. edit: first paragraph was edited for clarity |
|
kosmitek
Member #8,151
December 2006
|
I try change this code as you say (it is also in ATTACHMENT) - but when I changed it wrong run - for example if I press "M" then pawn doesn't run BUT when I next press "->" then it goes 2 times !
after change with following code my program wrong run:
also wrong run
is also wrong run |
|
LennyLen
Member #5,313
December 2004
|
If you're using the exact code that you've just put in those last three examples, then no, that won't work. That's just skeleton code to show you examples of how a loop can be done. You want something like this:
|
|
kosmitek
Member #8,151
December 2006
|
LennyLen your code also is wrong |
|
LennyLen
Member #5,313
December 2004
|
I didn't notice you were counting down i to give the player 20 moves. Here's a version that works exactly like the executable you included. I've restructured the program completely though, and changed parts that could be made simpler. You were also using the key[] array incorrectly, which is why you were getting multiple rolls each time you pressed a key. main.cpp:
player.h:
player.cpp:
|
|
kosmitek
Member #8,151
December 2006
|
Thanks LennyLen, but it is also wrong because: |
|
LennyLen
Member #5,313
December 2004
|
This new run_game() function "fixes" the problem of it rolling when other keys are pressed. I'll leave it up to you to figure out how to only display information when M is pressed.
|
|
BAF
Member #2,981
December 2002
|
Quote: I would think a much better design would be to have exit() call a "cleanup" function of the programmer's choice: Can't you use atexit() for that? |
|
kosmitek
Member #8,151
December 2006
|
WOOOW !!!!! Run very good - I must seeing through very attentively - thank you LennyLen -------------------------- |
|
LennyLen
Member #5,313
December 2004
|
Here's a version I rewrote that extends the player class so that it now stores what tile the player is on, as well as the result of the latest roll. I also added a method to reroll the dice. The program now relies less on global variables, and is a little more object oriented. I don't normally use C++, so there may be more efficient ways of doing things than I what I do. By the way, I've left the logic of your game untouched, as I'm not 100% certain what you're trying to do. I'm curious as to why there's a lot more code for when the player presses left then for when they press right. main.cpp: 1#include <iostream>
2#include <ctime>
3#include <allegro.h>
4#include "player.h"
5
6using namespace std;
7
8// Global variables
9// lots of them as I was too lazy to restructure the program properly
10BITMAP *board = NULL;
11BITMAP *pawn = NULL;
12BITMAP *buffer = NULL;
13
14
15// Function Declarations
16void initiate();
17void shutdown();
18void run_game();
19void display_screen(player *mike);
20
21// int main()
22int main()
23{
24 initiate();
25 run_game();
26 shutdown();
27 return 0;
28}
29END_OF_MAIN()
30
31
32// void initialize() - gets Allegro set up
33void initiate()
34{
35 allegro_init();
36 install_keyboard();
37
38 set_color_depth(32);
39 if (set_gfx_mode(GFX_AUTODETECT_WINDOWED,1024,768,0,0) != 0)
40 {
41 allegro_message("Error with gfx mode!");
42 exit(EXIT_FAILURE);
43 }
44
45 buffer=create_bitmap(SCREEN_W,SCREEN_H);
46
47 board = load_bitmap("board.bmp",NULL);
48 if (!board)
49 {
50 allegro_message("Error loading board.bmp!");
51 exit(EXIT_FAILURE);
52 }
53
54 pawn = load_bitmap("pawn.bmp",NULL);
55 if (!pawn)
56 {
57 allegro_message("Error loading pawn.bmp!");
58 exit(EXIT_FAILURE);
59 }
60
61 srand(time(0));
62}
63
64
65// void shutdown() - frees memory and exits Allegro
66void shutdown()
67{
68 destroy_bitmap(board);
69 destroy_bitmap(pawn);
70 destroy_bitmap(buffer);
71 allegro_exit();
72}
73
74
75// void run_game() - does main game loop
76void run_game()
77{
78 player theplayer(99, 99, 99, 99);
79
80 int module, press;
81
82 display_screen(&theplayer);
83
84 while (theplayer.get_moves() > 0)
85 {
86
87 press = readkey();
88
89 if ( (press >> 8) == KEY_LEFT)
90 {
91 theplayer.roll_dice();
92
93 if ( (theplayer.get_tile() >= 0) && (theplayer.get_tile() <= 4) )
94 {
95 if ( (theplayer.get_tile() - theplayer.get_roll()) >= 0)
96 theplayer.move_to_tile(theplayer.get_tile() - theplayer.get_roll());
97 else
98 theplayer.move_to_tile(9 - (theplayer.get_roll() - theplayer.get_tile() - 1));
99 }
100 else if ( (theplayer.get_tile() >= 5) && (theplayer.get_tile() <= 9) )
101 {
102 if ( (theplayer.get_tile() + theplayer.get_roll()) <= 9)
103 theplayer.move_to_tile(theplayer.get_tile() + theplayer.get_roll());
104 else
105 {
106 module = theplayer.get_roll() - (9 - theplayer.get_tile());
107 if (module < 0)
108 module =- module;
109 theplayer.move_to_tile(0 + module - 1);
110 }
111
112 }
113 display_screen(&theplayer);
114 }
115
116 if ( (press >> 8) == KEY_RIGHT)
117 {
118 theplayer.roll_dice();
119
120 if ( (theplayer.get_tile() >= 0) && (theplayer.get_tile() <= 4) )
121 theplayer.move_to_tile(theplayer.get_tile() + theplayer.get_roll());
122 else
123 theplayer.move_to_tile(theplayer.get_tile() - theplayer.get_roll());
124 display_screen(&theplayer);
125 }
126
127 if ( (press >> 8) == KEY_ESC) break;
128
129 }
130
131}
132
133
134// void display_screen() - updates and displays the buffer to the screen
135void display_screen(player *mike)
136{
137 int tab1[2][10]={
138 {100, 220, 370, 500, 700, 700, 500, 350, 220, 100},
139 {100, 100, 100, 100, 100, 340, 340, 340, 340, 340},
140 };
141
142 clear_to_color(buffer, makecol(255, 255, 255));
143 blit(board, buffer, 0, 0, 0, 0, board->w, board->h);
144 blit(pawn, buffer, 0, 0, tab1[0][mike->get_tile()], tab1[1][mike->get_tile()], pawn->w, pawn->h);
145 textprintf_ex(buffer, font, 267, 580, makecol(0, 0, 0), -1, "You have: %d money, %d health, %d weapon and %d power.", mike->get_money(), mike->get_health(), mike->get_weapon(), mike->get_power());
146 textprintf_ex(buffer, font, 267, 600, makecol(0, 0, 0), -1, "You throw: %d, it left you %d movements", mike->get_roll(), mike->get_moves());
147 textprintf_ex(buffer, font, 267, 620, makecol(0, 0, 0), -1, "-> - left, <- - right");
148 textprintf_ex(buffer, font, 267, 640, makecol(0, 0, 0), -1, "ESC - the end of the game");
149 blit(buffer, screen, 0, 0, 0, 0, SCREEN_W, SCREEN_H);
150}
player.h: 1#ifndef PLAYER_H
2#define PLAYER_H
3
4class player
5{
6private:
7 int money;
8 int health;
9 int weapon;
10 int power;
11 int moves;
12 int roll;
13 int tile;
14
15public:
16 player(int a, int b, int c, int d);
17 void roll_dice();
18 void move_to_tile(int x);
19 int get_money();
20 int get_health();
21 int get_weapon();
22 int get_power();
23 int get_moves();
24 int get_roll();
25 int get_tile();
26 int change_money(int x);
27 int change_health(int x);
28 int change_weapon(int x);
29 int change_power(int x);
30
31};
32
33#endif // PLAYER_H
player.cpp: 1#include <ctime>
2#include <iostream>
3#include "player.h"
4
5player::player(int a, int b, int c, int d)
6{
7 money = a;
8 health = b;
9 weapon = c;
10 power = d;
11 moves = 20;
12 roll = 0;
13 tile = 0;
14}
15
16void player::roll_dice()
17{
18 roll = 1 + rand() % 3;
19 moves--;
20}
21
22void player::move_to_tile(int x)
23{
24 tile = x;
25}
26
27int player::get_money()
28{
29 return money;
30}
31
32int player::get_health()
33{
34 return health;
35}
36
37int player::get_weapon()
38{
39 return weapon;
40}
41
42int player::get_power()
43{
44 return power;
45}
46
47int player::get_moves()
48{
49 return moves;
50}
51
52int player::get_roll()
53{
54 return roll;
55}
56
57int player::get_tile()
58{
59 return tile;
60}
61
62int player::change_money(int x)
63{
64 money = money + x;
65 return money;
66}
67
68int player::change_health(int x)
69{
70 health = health + x;
71 return health;
72}
73
74int player::change_weapon(int x)
75{
76 weapon = weapon + x;
77 return weapon;
78}
79
80
81int player::change_power(int x)
82{
83 power = power + x;
84 return power;
85}
|
|
kosmitek
Member #8,151
December 2006
|
LennyLen somewhere is little mistake - pawn after throw bone doesn't go after right fields - I tried change it but only when I use "goto" was okay |
|
LennyLen
Member #5,313
December 2004
|
Quote: LennyLen somewhere is little mistake - pawn after throw bone doesn't go after right fields - I tried change it but only when I use "goto" was okay The program I wrote behaves exactly as the one you wrote, so if there's a problem, it's in your logic. I've attached both your original program (I recompiled it to use the dynamic library so it was smaller) and the version I wrote so you can see for yourself that they behave the same.
|
|
TeamTerradactyl
Member #7,733
September 2006
|
Gee... This sounds almost like the program I provided earlier to you, kosmitek, before you added the "player" class. Can't for the LIFE of me figure out why that code couldn't be used... It was re-written to be very understandable, use the exact same keys, handle the exact same logic... Why not just extend what was already written for you and use that? It's not as if the code doesn't compile... LennyLen, I don't know if you saw that earlier post or not (in the other thread, no less), but you may be able to use it. If not... shrug.
|
|
kosmitek
Member #8,151
December 2006
|
TeamTerradactyl yes, but I didn't use your earlier program because he also was wrong |
|
Paul Pridham
Member #250
April 2000
|
OK... you guys are posting about how GOTO is ugly and hard to read, and then post this as some sort of counter-example: for(multiset<CObject*, SSortObjects>::iterator it = CObjectList::o().begin();it != CObjectList::o().end();) // loop through STL-multiset (could be any other container as well) if ((*it)->get_x() + (*it)->get_w() < CCamera::o().get_x()) // check, if some statement about the object is true and delete it { it = CObjectList::o().erase(it); // delete one object from the container } else it++;
I mostly use GOTOs for function-level error handling. Setting a flag, checking it in multiple points to break a loop/nest is slower and less efficient than a goto. ---- |
|
Bob
Free Market Evangelist
September 2000
|
Quote: You guys are posting about how GOTO is ugly and hard to read, and then post this as some sort of counter-example: Indeed. That code should be: typedef std::multiset<CObject*, SSortObjects> myset_t; bool condition(myset_t::value_type &v) { return (v.get_x() + v.get_w() < CCamera::o().get_x()); } myset_t &myset = CObjectList::o(); myset.erase(std::remove_if(myset.begin(), myset.end(), condition), myset.end()); Somewhat cleaner now, although it's artificially lengthened by the additional typedefs and statement splittings. -- |
|
kosmitek
Member #8,151
December 2006
|
I think in my program must be "goto", without "goto" my program isn't run good. LENNY LEN - YOUR IDEA WITH 3 files: main.cpp; player.h; player.cpp, one table; and globall variables ARE WONDERFUL - WITHOUT THAT I MUSTED (MUST IN THE PAST ?) WRITE EVERYTHING IN ONE BIG FUNCTION AND NOW I HAVE MANY LITTLE FUNCTIONS - THANK YOU - THIS IDEA IS VERYYYYY GOOD - THANK YOU LENNY LEN FOR YOUR IDEA - I USE YOUR IDEA (but I had use "goto") AND I ADD GLOBAL TABLE - ONLY ONE TABLE !!! - THANKS YOU !!!!!!!!! YOU ARE WONDERFUL - THANK YOU !!!!! |
|
Simon Parzer
Member #3,330
March 2003
|
Quote: MUST IN THE PAST ? had to Example: Because I forgot to turn off capslock I had to slap myself in the face several times. |
|
LennyLen
Member #5,313
December 2004
|
Quote: LennyLen, I don't know if you saw that earlier post or not (in the other thread, no less), but you may be able to use it. If not... shrug. Yeah I saw it. Most of my code was actually from when I rewrote the program last time kosmitek posted about it a few weeks ago. Quote:
TeamTerradactyl yes, but I didn't use your earlier program because he also was wrong The real problem was that you didn't explain very well what you were trying to do, and it's hard to tell from your code exactly what you are doing. Now that I'm pretty certain that I know what you're trying to do, I've changed the logic to reflect this. I've attached the working code (the changes were to the run_game() function) and an executable. There's still no need for goto. edit: Don't rely too much on global variables. They certainly have their uses, but if you have too many of them, it makes the program harder to follow and less modular. There's usually a more elegant solution.
|
|
ImLeftFooted
Member #3,935
October 2003
|
I think gotos can add much needed clarity to complex operations and I honestly think they are the shit. goto Failure is about as clear as you can get. There are a number of other completely valid and wonderfully clear uses of gotos. The ability for a language feature to be misused does not render the feature obsolete. |
|
kosmitek
Member #8,151
December 2006
|
now it runs ideal WITHOUT "GOTO" - LENNY LEN YOU ARE GENIUS !!!!!! |
|
BAF
Member #2,981
December 2002
|
Your shift key is sticking again. |
|
FMC
Member #4,431
March 2004
|
Thanks for the pretty code Bob, i didn't know you could handle it that way! What i previously did: //fl...ake is a vector iterator of the same type as snow //snow is a vector for(fl = snow.begin(); fl!=snow.end(); fl++){ if(fl->dead()){ snow.erase(fl); fl--; } } Is this wrong or bug prone? [FMC Studios] - [Caries Field] - [Ctris] - [Pman] - [Chess for allegroites] |
|
CGamesPlay
Member #2,559
July 2002
|
Quote: Is this wrong or bug prone? Yes, fl is invalid when it is erased: you can't increment or decrement. In practice, however, it will work for vectors so long as you are not erasing the final element. -- Ryan Patterson - <http://cgamesplay.com/> |
|
|
|