How bad is a goto statement?
HardTranceFan

Before anyone thinks about flaming me, no, I don't use goto myself. This comment got me thinking about it.

Assembler (Z80) has various jump instructions, some conditional, followed by the memory location to jump to. These differed to a call instruction in that once you jumped, you couldn't return (the address jumped from wasn't pushed onto a stack, whereas the calling address was).

With this in mind, is it wrong to use the occasional goto in code, other than for aesthetic reasons?

Jonatan Hedborg

I have to admit that i have used in on occasion, when the alternative was to re-write a loop and add a bunch of variables and if's. Not in a large program however.

It's just very ugly and makes it hard to read the code

Richard Phipps

It's not wrong to use it were it is appropriate.

TeamTerradactyl

My ideas:

I would say GOTO is a BadThing for 99% of the cases; most programmers could code well enough to never need something like this.

I have seen a GOTO in a severely-nested function, though, which seemed to do alright. It was inside, I think, 8-9 while() and for() loops, and the point of the GOTO was "Okay, the value is now true/false. Continue with the rest of the current function!" Otherwise, he would have had to test on EVERY loop whether "done == true" (or whatever), and that was too expensive when you're dealing with a loop that was close to O(2^10) or something like that.

So my answer is that you should avoid it when possible, but if you HAVE to use it, do it only for efficiency reasons like that above.

Matthew Leverton

In high level languages the use of goto can almost always be replaced by some other, more elegant solution. The only place in C where I ever use goto is to break out of some large nested loop. With some other languages, you can use the syntax break label; to avoid using a goto in such a scenario as that. Even in C, it's quite possible that such a loop can be better written as a function call that returns in place of the goto.

TeamTerradactyl

Matthew: I hadn't heard of "break label" before... That would be a useful tool if I ever start nesting... :D

Matthew Leverton

Both Java and D use break label;. PHP uses break level;, where level is the number of loops of which you want to break out.

ReyBrujo

Goto makes programs hard to read and analyze. The only use for goto is to break from nested cycles, yes, but most times you can prevent that by adding control variables to each of the loops.

Rampage

Goto can also be used to implement exception-like logic in languages like C. I remember having seen file access routines which had lots of potential failure points. Instead of repeating error handling code for each point, the routines used goto, which made them easier to understand. :o

Goalie Ca

A goto is only bad because people get lazy. Most of the time a goto is not needed (mostly only needed in low-level). Without the goto statement c and c++ are turing complete. The danger is someone could use a goto and avoid deep thinking and proper organization of the code (create a proper state machine and code it cleanly!). This is when it becomes hard/impossible to read and debug.

TeamTerradactyl

My question is this: If you use a GOTO to break out of a function, what happens to all the local variables? Are they taken out of scope, or can they still be accessed?

Oh, and I think I should start doing this from now on:

1int main(int argc, char **argv)
2{
3 string temp;
4 for (int i = 0; i < argc; i++)
5 {
6 temp = argv<i>;
7 if (temp == "--help")
8 {
9 goto HELP;
10HELPDONE:
11 }
12 else if (temp == "--version")
13 {
14 goto VERSION;
15VERSIONDONE:
16 }
17 else if (temp == "--screenResolution")
18 {
19 goto RESOL
20RESOLDONE:
21 }
22 else if (...)
23 }
24 
25 goto END;
26 
27HELP:
28 cout << "You asked for help...";
29 goto HELPDONE;
30VERSION:
31 cout << "You're asking for the version...";
32 goto VERSIONDONE;
33RESOL:
34 cout << "You're asking for the screen resolution...";
35 goto RESOLDONE
36 
37END:
38 return 0;
39}

EDIT: Fixed the labels per Simon Parzer's feedback. Thanks, Simon! :)

Rampage

In C goto can't take you outside the scope of the current function.

Quote:

Oh, and I think I should start doing this from now on:

Oh no! Don't ever do that! That's what the switch clause is for.

Simon Parzer

Why don't you try it?

You got it all wrong BTW...

You do labels and gotos in C like this:

ThisIsALabel:;
//some code
goto ThisIsALabel;

I don't know exactly what you posted but I think the syntax is wrong.
In my opinion you should use goto where you need it. As long as you don't plan to share your code with others.
Sometimes it even makes code simpler. I mean, I rather have a bunch of gotos and three labels than five nested loops. When I started with BASIC a long time ago I got used to GOTOs and I have no big problems with them now.
In C++ or modular C the goto statement doesn't make much sense anymore, though.

HardTranceFan

Hey TT, your code put a grin on my face. ;D

orz

Sometimes, when writing my flow control, its difficult to implement it or think of it in terms of the standard C/C++ flow control primitives of for/while/do/switch loops and function calls. Usually in such cases I'll write what I'm thinking of in terms of loops and function calls, but do most of the flow control with if...break; and if...continue; statements and a few variables declared for the sole purpose of acting as flags for the loops.

Occaisonally though, the code will become harder to maintain, and/or debug if I write it that way, so I'll use goto statements instead. That has only happened maybe 10-20 times in the last 30 thousand lines of C/C++ code I've written, and some of those cases could have been expressed as exceptions more naturally if I hadn't been trying to avoid some issue involved with exceptions in the relevant code.

Even more rarely, sometimes I'll write something where I have such a difficult time even concieving of the flow control necessary that expressing it in terms of anything other than the natural flow control primitives I concieved of it in (often gotos for something like that) would take a great deal of diagramming and analysis and just be asking for trouble, so I write it as I concieve of it. That has only happened maybe 2 or 3 times in the last 30 thousand lines of C/C++ code I've written. Perhaps I should spend the time and effort diagramming and analyzing such cases; perhaps it would improve my ability to think in terms of the usual flow control primitives. But I'm not too sure about that. Perhaps that would merely limit my thinking to things that can easily be expressed in C/C++ instead.

Rick

I have never used goto in C/C++, and the only time I've ever used it is with VB 6 for error handling. Because I never use it, I never think about it, and therefore when coding, my mind never even considers it.

Audric

Ladies and gentlemen, I give you: the computed goto in C.
The &&X syntax is a DJGPPism for : "the memory address of label X".

1#define AMC_GOTO_PCODE goto *pcode[*code_pointer]
2 
3 static const void * pcode[] = {
4 NULL,
5 &&AMCInt, // 1
6 &&AMCRegGlobal,// 2
7 &&AMCRegLocal, // 3
8 &&AMCRnd, // 4
9 &&AMCJump, // 5
10 &&AMCDirect, // 6
11 &&AMCExit, // 7
12 &&AMCIfnot, // 8
13 &&AMCIfjump, // 9
14 &&AMCIfdirect, // 10
15 &&AMCIfexit, // 11
16 &&AMCEnd, // 12
17 &&AMCAuto, // 13
18 &&AMCStore, // 14
19 &&AMCRecall, // 15
20 &&AMCStackup, // 16
21 &&AMCSpawn, // 17
22 &&AMCPause, // 18
23 &&AMCMove // 19
24 };
25 
26(...)
27// start of interpretation
28AMC_GOTO_PCODE;
29 
30(...) // some sample instructions :
31AMCInt: // put an Integer in the register
32 code_pointer++;
33 AmEval(&currentexp,code_pointer,*(code_pointer+1));
34 code_pointer+=2;
35 AMC_GOTO_PCODE;
36 
37AMCJump: // Jump
38 code_pointer++;
39 code_pointer= *code_pointer + BOB[Actor].programstart;
40 AMC_GOTO_PCODE;

Compared to a switch() statement, this was way cooler IMHO, and the emulation time went from 3% to 2% (precision of this figure is low, though)

Johan Halmén

I've never used goto. I do think it is a bad thing. The more you use goto, the more your code differs from what is considered typical C code structure. As Matthew said, you can put the part of code in a function and have returns instead.

TeamTerradactyl

Johan, I would agree with that, unless there has to be a lot of data transfer.

Say, for example, that you have declared a bunch of data at the beginning of your function. Then, you want to test whether "a == b" and "c < d" or "e != f", etc. If you push all that data into a separate function just to do the test, it slows down the code quite a bit.

If, however, you break out of a very-nested group of loops with a GOTO, you can continue with all the data and variables you had declared/changed/etc. as if you had simply "break"-ed a bunch of times to get there...

Kitty Cat

Here's some code I have for decoding macroblocks from an MPEG-1 data stream:

1static void p_picture(APEG_LAYER *layer)
2{
3 const int MBAmax = layer->mb_cols*layer->mb_rows;
4 int macroblock_type;
5 int coded_block_pattern;
6 int MBA, MBAinc;
7 int dc_dct_pred[3];
8 int PMV[2];
9 int bx, by;
10 unsigned int code;
11 
12slice_start:
13 dc_dct_pred[0] = dc_dct_pred[1] = dc_dct_pred[2] = 0;
14 PMV[0] = PMV[1] = 0;
15 
16 code = apeg_start_code(layer);
17 if(code < SLICE_START_CODE_MIN || code > SLICE_START_CODE_MAX)
18 return;
19 apeg_flush_bits32(layer);
20 
21 slice_header(layer);
22 
23 bx = get_mba_inc(layer);
24 by = (code&255) - 1;
25 
26 MBA = by*layer->mb_cols + bx;
27 
28 bx <<= 4;
29 by <<= 4;
30 
31block_start:
32 code = show_bits(layer, 6);
33 
34 if(code >= 8)
35 {
36 code >>= 3;
37 
38 apeg_flush_bits8(layer, PMBtab0[ code ].len);
39 macroblock_type = PMBtab0[ code ].val;
40 
41 dc_dct_pred[0] = dc_dct_pred[1] = dc_dct_pred[2] = 0;
42 
43 switch(macroblock_type & (MACROBLOCK_MOTION_FORWARD|MACROBLOCK_PATTERN))
44 {
45 case MACROBLOCK_MOTION_FORWARD|MACROBLOCK_PATTERN:
46 apeg_motion_vector(layer,PMV,forward_code,full_forward_vector);
47 apeg_form_f_pred(layer, bx, by, PMV);
48 
49 break;
50 
51 case MACROBLOCK_PATTERN:
52 PMV[0] = PMV[1] = 0;
53 apeg_empty_pred(layer->forward_frame, layer->current_frame, bx, by, layer->coded_width);
54 
55 break;
56 
57 default:
58 apeg_motion_vector(layer,PMV,forward_code,full_forward_vector);
59 apeg_form_f_pred(layer, bx, by, PMV);
60 
61 goto next;
62 }
63 }
64 else
65 {
66 apeg_flush_bits8(layer, PMBtab1[ code ].len);
67 macroblock_type = PMBtab1[ code ].val;
68 
69 switch(macroblock_type & (MACROBLOCK_QUANT|MACROBLOCK_INTRA))
70 {
71 case MACROBLOCK_QUANT|MACROBLOCK_INTRA:
72 layer->quantizer_scale = apeg_get_bits8(layer, 5);
73 // fall-through...
74 case MACROBLOCK_INTRA:
75 PMV[0] = PMV[1] = 0;
76 
77 apeg_decode_intra_blocks(layer, dc_dct_pred);
78 
79 apeg_fast_idct(apeg_block[0]);
80 apeg_fast_idct(apeg_block[1]);
81 apeg_fast_idct(apeg_block[2]);
82 apeg_fast_idct(apeg_block[3]);
83 apeg_fast_idct(apeg_block[4]);
84 apeg_fast_idct(apeg_block[5]);
85 
86 Move_Blocks(layer, bx, by);
87 
88 goto next;
89 
90 case 0: // Table 1 must have Intra and/or Quant flags set
91 goto slice_start;
92 }
93 
94 layer->quantizer_scale = apeg_get_bits8(layer, 5);
95 dc_dct_pred[0] = dc_dct_pred[1] = dc_dct_pred[2] = 0;
96 
97 if(macroblock_type & MACROBLOCK_MOTION_FORWARD)
98 {
99 apeg_motion_vector(layer,PMV,forward_code,full_forward_vector);
100 apeg_form_f_pred(layer, bx, by, PMV);
101 }
102 else
103 {
104 PMV[0] = PMV[1] = 0;
105 apeg_empty_pred(layer->forward_frame, layer->current_frame, bx, by, layer->coded_width);
106 }
107 }
108 
109 coded_block_pattern = get_block_pattern(layer);
110 
111 do_block(0);
112 do_block(1);
113 do_block(2);
114 do_block(3);
115 do_block(4);
116 do_block(5);
117 
118next:
119 if(++MBA >= MBAmax)
120 return;
121 
122 if(show_bits(layer, 24) == 1)
123 goto slice_start;
124 
125 MBAinc = get_mba_inc(layer);
126 if(MBAinc != 0)
127 {
128 int i = MBAinc;
129 
130 dc_dct_pred[0] = dc_dct_pred[1] = dc_dct_pred[2] = 0;
131 PMV[0] = PMV[1] = 0;
132 
133 do {
134 if((bx += 16) == layer->coded_width)
135 {
136 bx = 0;
137 by += 16;
138 }
139 apeg_empty_pred(layer->forward_frame, layer->current_frame, bx, by, layer->coded_width);
140 } while(--i);
141 
142 MBA += MBAinc;
143 if(MBA >= MBAmax)
144 return;
145 }
146 
147 if((bx += 16) == layer->coded_width)
148 {
149 bx = 0;
150 by += 16;
151 }
152 
153 goto block_start;
154}

It's pretty clear to see the code flow, IMO. 'goto block_start', and it goes to decode the beginning of a block. 'goto slice_start', and it goes to start decode the beginning of a series of blocks. 'goto next', and it goes to find the next block of the slice.

Compare that with a double nested loop (adding two more indentations to a good portion of the code), extra if checks, ambiguous 'continue' and 'break' commands, and needing to trace close brackets to see where something loops back to..

PS: Whee, having PMBtab0[code] in my code breaks the syntax parser. :D

Billybob
Quote:

How bad is a goto statement?

goto jail;

tobing

We have managed to produce way over one million lines of C++ code with the only goto statements being in the various implementations of the quicksort algorithm. So I would advocate to not use goto, except when it makes algorithms easier to read, which is most often not the case. So, IF the algorithm is easier to read and understand using goto, then use it (but with great care!)...

Kris Asick

When I was still learning how to program I would constantly use goto. Especially when I was first learning BASIC because the gosub command didn't make sense to me.

Learning C/C++ eventually taught me why goto was stupid for most situations.

But, I do still use it, though only in one very specific situation: To restart a procedure. My goto statements always return an executing procedure right back to where it started, or as close as is necessary for the purpose at hand, thus eliminating the need to have multiple nested do loops or recursion.

Goto is generally avoidable, but in the uncommon chance that it makes your code MORE readable than a do loop or if statement, then by all means, use it.

--- Kris Asick (Gemini)
--- http://www.pixelships.com

HoHo
Quote:

goto jail;

When I was coding in QBasic I used to have label for a part of code that printed some generic error message and exited the program. It was always called hell :)

raccoon

I think Sun has a good reason to not even include the goto-statement in Java.

Joel Pettersson

It is seldom needed, and I don't use them particularly often, but goto statements can occasionally improve both efficiency and clarity. Once in a rare while they are the best solution; when this is the case, it is nothing short of stupid to religiously avoid them. Whatever the best tool for the job is, use it.

imaxcs

Consider this code:

handle_objectdeletion:
for(multiset<CObject*, SSortObjects>::iterator it = CObjectList::o().begin();it != CObjectList::o().end();it++) // 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
      {
        CObjectList::o().remove(it); // delete one object from the container
        goto handle_objectdeletion; // we must break from the loop as the iterators might be invalid now...
      }

Got any advice to make it better?

HoHo

use break instead of goto and put it inside two loops, outer one loops until it has reached the end of the container.

Something like this (grossly simplified to make things clearer):

1bool needsDelete=false;
2vector<Data> data;
3vector<Data>::iterator iter;
4while(1){
5 for (iter=data.begin(); iter!=data.end(); iter++){
6 if (iter.needs_to_be_deleted()){
7 needsDelete=true;
8 // found something needing removal, break from for loop.
9 break;
10 }
11 }
12 if (needsDelete){
13 iter.remove();
14 needsDelete=false;
15 } else {
16 // looped through every variable, nothing needs deletion, breaks from while(1)
17 break;
18 }
19}

Code not tested but in theory it should work. There are about a million other ways of doing the same thing.

[edit]
Of course if the container is big then some kind of system should be used that would prevent the inner for loop to start over every time something gets deleted.

Audric

If you want to remove all the items that match the test:

multiset<CObject*, SSortObjects>::iterator it;
it = CObjectList::o().begin();
while(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
      {
        CObjectList::o().remove(it); // delete one object from the container
        it = CObjectList::o().begin(); // restart the whole loop
      }
      it++;
}

In this case, it's possible because "it = CObjectList::o().begin()" is a clean "restart", and if you delete the last element it matches the end-of-loop condition.

CGamesPlay

Ugh, learn the STL ::)

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++;

Audric

The docs I found say erase() returns void :-/

My method of restarting is inefficient. Better to memorize a reference to the element to remove, then increase the iterator, then remove the to_be_removed element.

edit: anyway, multiset is probably a bad example, as it's one of the STL containers which does not invalidate iterators on remove.

Jakub Wasilewski
Quote:

The docs I found say erase() returns void :-/

Your docs are probably outdated.

Audric
Jakub Wasilewski

Didn't read through carefully, didn't notice that we're talking about multisets here. Sorry Audric, you're right about that.

To clarify, multisets are AssociativeContainers, and those have a void erase() (they don't need the return value anyway). The iterator erase() is part of Sequence (which is a list, vector, deque etc.). My comment about outdated docs stemmed from the fact that in the past, even Sequences had a void erase() in some compilers.

Audric
Quote:

With this in mind, is it wrong to use the occasional goto in code, other than for aesthetic reasons?

Once you start down the dark path, forever will it dominate your destiny, consume you it will.
-- Yoda

Personnally, I use it's much wronger to write things like:

   // (inside main, for example)
   if (some big problem in initialization)
   {
      // exception handling
      allegro_exit();
      exit(0);
   }
   else
   {
      // a few hundred lines here....
   }
   // ...continued here... for some reason. argh!
}

orz
Quote:

To clarify, multisets are AssociativeContainers, and those have a void erase() (they don't need the return value anyway). The iterator erase() is part of Sequence (which is a list, vector, deque etc.). My comment about outdated docs stemmed from the fact that in the past, even Sequences had a void erase() in some compilers.

IIRC, erase() returns void on associative containers UNLESS you're using microsofts implementation and/or documentation : )

TeamTerradactyl

Audric's statement gave me a question:

void some_function_other_than_main()
{
  ...
  if (error)
    exit(1); // <-- This line
}

Does the "exit(1)" do about the same thing as the following code (barring the fact that "goto" was not supposed to leave the current function, or that "exit()" passes a value to be returned):

void some_function_other_than_main()
{
  ...
  if (error)
    goto End_of_Main_Function; // <-- This line
}

int main()
{
  ...
:End_of_Main_Function
  return 0;
}

So if you don't have a chance to clean up a bunch of your dynamically-allocated arrays before your program does a nose-dive, they will NEVER reach the destructors for your classes and/or cleanup() code?

CGamesPlay
Quote:

So if you don't have a chance to clean up a bunch of your dynamically-allocated arrays before your program does a nose-dive, they will NEVER reach the destructors for your classes and/or cleanup() code?

All destructors on objects will be called with exit. They will not with abort, though.

TeamTerradactyl
CGamesPlay said:

All destructors on objects will be called with exit. They will not with abort, though.

But only if the dynamic array was created inside a class (which has a proper destructor)?

int main()
{
  int *theArray;
  theArray = new int[400];

  exit(1);
};

So here, you would have a memory leak, or so I would imagine.

I would think a much better design would be to have exit() call a "cleanup" function of the programmer's choice:

1int clean_all_my_stuff(int wasErrorOnExit)
2{
3 destroy_bitmap(double_buffer);
4 delete [] globalArray;
5 
6 return wasErrorOnExit;
7}
8 
9int main()
10{
11 ...
12 if (error)
13 exit(clean_all_my_stuff(1));
14 
15 ...
16 return (clean_all_my_stuff(0)); // <-- Passes "0" to signify all went well
17}

HoHo
Quote:

So if you don't have a chance to clean up a bunch of your dynamically-allocated arrays before your program does a nose-dive, they will NEVER reach the destructors for your classes and/or cleanup() code?

I never use exit. Errors that force program shutdown usually appear while setting up the program. I just create a function for initialization that returns some value. If it is an error it returns an error value (duh!). When I see that an error occured I simply call the function that is responsible for cleaning up and return from main afterwards.

kosmitek

very interesting subject

example:

1while(....)
2{
3 
4 
5INSTRUCTION;
6 
7here:
8if()
9{}
10else if ()
11{}
12 
13else()
14{
15goto: here;
16}
17 
18 
19}

How should I delete goto ? In above example - somebody must do something, he won't go further if he won't do something, and if I delete "goto" then instructions be become executed and I don't want it - have you solution ?

Jonatan Hedborg
1while(....)
2{
3INSTRUCTION;
4 
5 do {
6 bool repeat = false;
7 if()
8 {}
9 else if ()
10 {}
11 else()
12 {
13 repeat = true;
14 }
15 } while( repeat );
16}

for example.

Matthew Leverton
1while(....)
2{
3 INSTRUCTION;
4 do
5 {
6 if()
7 {
8 // blah
9 }
10 else if()
11 {
12 // blah
13 }
14 else()
15 {
16 continue;
17 }
18 } while (FALSE);
19}

TeamTerradactyl

kosmitek: both Matthew and Jonatan are correct: you would use some sort of loop (while, for, etc.) until you specify that you should break out of it (or, you can specify that you will only run it once UNLESS some condition is met).

1bool breakInsideLoop = false;
2while(...)
3{
4 EXPRESSION;
5 
6 while (!breakInsideLoop)
7 {
8 ...
9 
10 if (...)
11 breakInsideLoop = true;
12 }
13 
14 ... // Now, outside of the "inside" loop
15}

Goalie Ca

Actually i've just been reading/writting some assembly and for some reason this version seems a lot nicer. I guess it depends on the audience and the style of programming and what mood your in.

here:
if()
{}
else if ()
{}

else()
{
goto: here;
}

but matts' version is quite nice as well. The adding an extra variable is kind of a nasty hack.

Richard Phipps

This is making my head hurt! :o

TeamTerradactyl

I guess we can all just start learning low-level programming and use the goto: statement a lot more ... :D "Let's see... push aex onto, uh... fex... then pop foo off of bar... Whoops; that should have been fee from bak. Where's my debugger again..?"

Matthew Leverton
Quote:

Actually i've just been reading/writting some assembly and for some reason this version seems a lot nicer.

But it really is a loop—it's not a one time jump. I prefer the do/while construct because of that. Ideally there would be a slimmer syntax like:

do {
  // ...
  if (foo) continue;
  // ...
}

Either way, the use of goto (or a similar loop construct) usually indicates that you should consider rethinking your approach altogether. I think the biggest problem with relying on goto for anything is that you might end up relying on it elsewhere out of laziness.

Arthur Kalliokoski

Winduhs search just found 41 gotos in Allegro source tree on this computer...

Matthew Leverton

Most of Allegro's goto usage is for error handling. Given C's lack of exceptions, there's not always a good alternative.

Sirocco

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

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.
(/me is mostly joking)

edit: first paragraph was edited for clarity

kosmitek

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 !

1 
2#include<iostream>
3#include<ctime>
4#include <allegro.h>
5using namespace std;
6 
7 
8 
9 
10 
11class player
12{
13 private:
14 int money;
15 int health;
16 int weapon;
17 int power;
18
19
20 public:
21 player(int a, int b, int c, int d)
22 {
23 money=a;
24 health=b;
25 weapon=c;
26 power=d;
27 }
28
29 int change_money(int x)
30 {
31 money=money+x;
32 return money;
33 }
34
35 int change_health(int x)
36 {
37 health=health+x;
38 return health;
39 }
40
41
42 int change_weapon(int x)
43 {
44 weapon=weapon+x;
45 return weapon;
46 }
47
48
49 int change_power(int x)
50 {
51 power=power+x;
52 return power;
53 }
54
55~player() {}
56
57};
58 
59 
60 
61 
62 
63void start_game(BITMAP *board, BITMAP *pawn, BITMAP *buffer)
64{
65
66int tab1[2][10]={
67 {100,220,370,500,700,700,500,350,220,100},
68 {100,100,100,100,100,340,340,340,340,340},
69 };
70 
71int tab2[10]={0,1,2,3,4,5,6,7,8,9};
72 
73int *index=&tab2[0];
74int *zero=&tab2[0];
75int *nine=&tab2[9];
76int module;
77int eyes;
78srand (time(0));
79int i=21;
80int y=1;
81 
82player game(99, 99, 99, 99);
83 
84while(!key[KEY_ESC]&&i!=0)
85{
86i--;
87eyes = 1+rand()%3;
88clear_to_color(buffer,makecol(255,255,255));
89blit(board,buffer,0,0,0,0,board->w,board->h);
90blit(pawn,buffer,0,0,tab1[0][*index],tab1[1][*index],pawn->w,pawn->h);
91textprintf_ex(buffer, font, 267, 580, makecol(0, 0, 0), -1, "You have: %d money, %d health, %d weapon and %d power.", game.change_money(y), game.change_money(y), game.change_weapon(y), game.change_power(y));
92textprintf_ex(buffer, font, 267, 600, makecol(0, 0, 0), -1, "You throw: %d, it left yo %d movements", eyes, i);
93textprintf_ex(buffer, font, 267, 620, makecol(0, 0, 0), -1, "-> - left, <- - right");
94textprintf_ex(buffer, font, 267, 640, makecol(0, 0, 0), -1, "ESC - the end of the game");
95blit(buffer,screen,0,0,0,0,SCREEN_W,SCREEN_H);
96
97 
98 
99 
100direction:
101readkey();
102if (key[KEY_LEFT])
103{
104if(*index>=0&&*index<=4)
105 {
106 if ((*index-eyes)>=*zero)
107 {
108 index=index-eyes;
109 }
110 else
111 {
112
113 index=nine-(eyes-*index-1);
114 }
115
116 }
117else if(*index>=5&&*index<=9)
118 
119 {
120 if ((*index+eyes)<=*nine)
121 {
122 index=index+eyes;
123 }
124 else
125 {
126 module=eyes-(*nine-*index);
127 if (module<0)
128 {
129 module=-module;
130 }
131 index=zero+module-1;
132 }
133
134 }
135}
136 
137 
138 
139
140else if (key[KEY_RIGHT])
141{
142 if(*index>=0&&*index<=4)
143 {
144 index=index+eyes;
145
146 }
147 else
148 {
149 index=index-eyes;
150 }
151
152
153}
154
155
156
157else
158{
159 if (!key[KEY_ESC])
160 {
161 goto direction;
162 }
163
164}
165 
166 
167 
168}
169 
170delete game;
171
172
173}
174 
175 
176 
177 
178 
179 
180int main()
181{
182BITMAP *board = NULL;
183BITMAP *pawn = NULL;
184BITMAP *buffer = NULL;
185allegro_init();
186install_keyboard();
187set_color_depth(32);
188set_gfx_mode(GFX_AUTODETECT,1024,768,0,0);
189buffer=create_bitmap(SCREEN_W,SCREEN_H);
190 
191 
192board = load_bitmap("board.bmp",NULL);
193pawn = load_bitmap("pawn.bmp",NULL);
194 
195start_game(board, pawn, buffer);
196 
197 
198destroy_bitmap(board);
199destroy_bitmap(pawn);
200destroy_bitmap(buffer);
201allegro_exit();
202return 0;
203}
204END_OF_MAIN()

after change with following code my program wrong run:

1 
2while(....)
3{
4INSTRUCTION;
5 
6 do {
7 bool repeat = false;
8 if()
9 {}
10 else if ()
11 {}
12 else()
13 {
14 repeat = true;
15 }
16 } while( repeat );
17}

1while(....)
2{
3 INSTRUCTION;
4 do
5 {
6 if()
7 {
8 // blah
9 }
10 else if()
11 {
12 // blah
13 }
14 else()
15 {
16 continue;
17 }
18 } while (FALSE);
19}

also wrong run

1bool breakInsideLoop = false;
2while(...)
3{
4 EXPRESSION;
5 
6 while (!breakInsideLoop)
7 {
8 ...
9 
10 if (...)
11 breakInsideLoop = true;
12 }
13 
14 ... // Now, outside of the "inside" loop
15}

is also wrong run

LennyLen

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:

1int done = 0;
2 
3while (!done)
4{
5 
6 if (key[KEY_LEFT])
7 {
8 if(*index>=0&&*index<=4)
9 {
10 if ((*index-eyes)>=*zero)
11 {
12 index=index-eyes;
13 }
14 else
15 {
16 
17 index=nine-(eyes-*index-1);
18 }
19 
20 }
21
22 else if(*index>=5&&*index<=9)
23 {
24 if ((*index+eyes)<=*nine)
25 {
26 index=index+eyes;
27 }
28 else
29 {
30 module=eyes-(*nine-*index);
31 if (module<0)
32 {
33 module=-module;
34 }
35 index=zero+module-1;
36 }
37 
38 }
39
40 }
41 
42 else if (key[KEY_RIGHT])
43 {
44 if(*index>=0&&*index<=4)
45 {
46 index=index+eyes;
47 
48 }
49 else
50 {
51 index=index-eyes;
52 }
53 
54 }
55 
56 else if (key[KEY_ESC]) done = 1;
57 
58}

kosmitek

LennyLen your code also is wrong :) Pawn don't want go and it is reason why I use "goto" here, every other method is wrong.

LennyLen

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:

1#include <iostream>
2#include <ctime>
3#include <allegro.h>
4#include "player.h"
5 
6 
7using namespace std;
8 
9 
10// Global variables
11// lots of them as I was too lazy to restructure the program properly
12BITMAP *board = NULL;
13BITMAP *pawn = NULL;
14BITMAP *buffer = NULL;
15int module, eyes, y = 1, index = 0, moves = 20;
16player game(99, 99, 99, 99);
17 
18 
19// Function Declarations
20void initiate();
21void shutdown();
22void run_game();
23void display_screen();
24 
25 
26// int main()
27int main()
28{
29 initiate();
30 run_game();
31 shutdown();
32 return 0;
33}
34END_OF_MAIN()
35 
36 
37// void initialize() - gets Allegro set up
38void initiate()
39{
40 allegro_init();
41 install_keyboard();
42 
43 set_color_depth(32);
44 if (set_gfx_mode(GFX_AUTODETECT_WINDOWED,1024,768,0,0) != 0)
45 {
46 allegro_message("Error with gfx mode!");
47 exit(EXIT_FAILURE);
48 }
49 
50 buffer=create_bitmap(SCREEN_W,SCREEN_H);
51 
52 board = load_bitmap("board.bmp",NULL);
53 if (!board)
54 {
55 allegro_message("Error loading board.bmp!");
56 exit(EXIT_FAILURE);
57 }
58 
59 pawn = load_bitmap("pawn.bmp",NULL);
60 if (!pawn)
61 {
62 allegro_message("Error loading pawn.bmp!");
63 exit(EXIT_FAILURE);
64 }
65}
66 
67 
68// void shutdown() - frees memory and exits Allegro
69void shutdown()
70{
71 destroy_bitmap(board);
72 destroy_bitmap(pawn);
73 destroy_bitmap(buffer);
74 allegro_exit();
75}
76 
77 
78// void run_game() - does main game loop
79void run_game()
80{
81 
82 srand (time(0));
83 int press;
84 
85 while (moves > 0)
86 {
87 eyes = 1 + rand() % 3;
88 display_screen();
89 
90 press = readkey();
91 
92 if ( (press >> 8) == KEY_LEFT)
93 {
94 if ( (index >= 0) && (index <= 4) )
95 {
96 if ( (index - eyes) >= 0)
97 index = index - eyes;
98 else
99 index = 9 - (eyes - index - 1);
100 }
101 else if ( (index >= 5) && (index <= 9) )
102 {
103 if ( (index + eyes) <= 9)
104 index = index + eyes;
105 else
106 {
107 module = eyes - (9 - index);
108 if (module < 0)
109 module =- module;
110 index = 0 + module - 1;
111 }
112 
113 }
114 moves--;
115 }
116 
117 if ( (press >> 8) == KEY_RIGHT)
118 {
119 if ( (index >= 0) && (index <= 4) )
120 index = index + eyes;
121 else
122 index = index - eyes;
123 moves--;
124 }
125 
126 if ( (press >> 8) == KEY_ESC) break;
127 
128 }
129 
130}
131 
132 
133// void display_screen() - updates and displays the buffer to the screen
134void display_screen()
135{
136 int tab1[2][10]={
137 {100, 220, 370, 500, 700, 700, 500, 350, 220, 100},
138 {100, 100, 100, 100, 100, 340, 340, 340, 340, 340},
139 };
140 
141 clear_to_color(buffer, makecol(255, 255, 255));
142 blit(board, buffer, 0, 0, 0, 0, board->w, board->h);
143 blit(pawn, buffer, 0, 0, tab1[0][index], tab1[1][index], pawn->w, pawn->h);
144 textprintf_ex(buffer, font, 267, 580, makecol(0, 0, 0), -1, "You have: %d money, %d health, %d weapon and %d power.", game.change_money(y), game.change_money(y), game.change_weapon(y), game.change_power(y));
145 textprintf_ex(buffer, font, 267, 600, makecol(0, 0, 0), -1, "You throw: %d, it left you %d movements", eyes, moves);
146 textprintf_ex(buffer, font, 267, 620, makecol(0, 0, 0), -1, "-> - left, <- - right");
147 textprintf_ex(buffer, font, 267, 640, makecol(0, 0, 0), -1, "ESC - the end of the game");
148 blit(buffer, screen, 0, 0, 0, 0, SCREEN_W, SCREEN_H);
149}

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 
12public:
13 player(int a, int b, int c, int d);
14 int change_money(int x);
15 int change_health(int x);
16 int change_weapon(int x);
17 int change_power(int x);
18 
19};
20 
21#endif // PLAYER_H

player.cpp:

1#include "player.h"
2 
3player::player(int a, int b, int c, int d)
4{
5 money = a;
6 health = b;
7 weapon = c;
8 power = d;
9}
10 
11int player::change_money(int x)
12{
13 money = money + x;
14 return money;
15}
16 
17int player::change_health(int x)
18{
19 health = health + x;
20 return health;
21}
22 
23int player::change_weapon(int x)
24{
25 weapon = weapon + x;
26 return weapon;
27}
28 
29 
30int player::change_power(int x)
31{
32 power = power + x;
33 return power;
34}

kosmitek

Thanks LennyLen, but it is also wrong because:
please start this game, please press "m" or other letter on keyboard 5 or more times and you see that information: "You throw: X " - and that X(number of eyes) it will be change - and X(number of eyes) should change ONLY when somebody press "->" or "<-", when somebody press something else X(number of eyes) shouldn't change - it is a problem which can be solved only when wy use "goto" - that I think.

LennyLen

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.

1void run_game()
2{
3 
4 srand (time(0));
5 int press;
6 
7 display_screen();
8 
9 while (moves > 0)
10 {
11 eyes = 1 + rand() % 3;
12 
13 press = readkey();
14 
15 if ( (press >> 8) == KEY_LEFT)
16 {
17 if ( (index >= 0) && (index <= 4) )
18 {
19 if ( (index - eyes) >= 0)
20 index = index - eyes;
21 else
22 index = 9 - (eyes - index - 1);
23 }
24 else if ( (index >= 5) && (index <= 9) )
25 {
26 if ( (index + eyes) <= 9)
27 index = index + eyes;
28 else
29 {
30 module = eyes - (9 - index);
31 if (module < 0)
32 module =- module;
33 index = 0 + module - 1;
34 }
35 
36 }
37 moves--;
38 display_screen();
39 }
40 
41 if ( (press >> 8) == KEY_RIGHT)
42 {
43 if ( (index >= 0) && (index <= 4) )
44 index = index + eyes;
45 else
46 index = index - eyes;
47 moves--;
48 display_screen();
49 }
50 
51 if ( (press >> 8) == KEY_ESC) break;
52 
53 }
54 
55}

BAF
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

WOOOW !!!!! Run very good - I must seeing through very attentively - thank you LennyLen :)

--------------------------
ups - pawn after throw bone doesn't go after right fields - I tried change it but only when I use "goto" was okay :(

LennyLen

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:

#SelectExpand
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:

#SelectExpand
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:

#SelectExpand
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

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
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

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

TeamTerradactyl yes, but I didn't use your earlier program because he also was wrong :)
In my program when I throw for example 3, pawn go 3 fields and in yours programs pawn is strange going - somewhere in your programs is little mistake, I must use "goto" in your program and only then pawn going right.

Paul Pridham

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++;

::) :D

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
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

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 !!!!! ;D;D

Simon Parzer
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
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
In my program when I throw for example 3, pawn go 3 fields and in yours programs pawn is strange going

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.

Dustin Dettmer

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

now it runs ideal WITHOUT "GOTO" - LENNY LEN YOU ARE GENIUS !!!!!!

BAF

Your shift key is sticking again. :-/

FMC

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?

CGamesPlay
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.

TeamTerradactyl
FMC said:

for (fl = snow.begin(); fl!=snow.end(); fl++)
{
  if (fl->dead())
  {
    snow.erase(fl);
    fl--; // <-- I think this is invalid
  }
}

If I'm not mistaken, the vector iterator only has the ++ operator defined; not the --, so you could not decrement your current position...

Dustin Dettmer
Quote:

//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?

Yes. This is what you want to do.

//fl...ake is a vector iterator of the same type as snow
//snow is a vector
for(fl = snow.begin(); fl!=snow.end(); ){
    if(fl->dead()){
        fl = snow.erase(fl);
    }
    else fl++;
}

Bob

How about this?

snow.erase(std::remove_if(snow.begin(), snow.end(), std::mem_fun(&snow_type::dead)),
           snow.end());

(Untested code)

Dustin Dettmer

Cool :D, I'm totally gonna use that.

FMC

Thanks for the head-ups!

Thread #589282. Printed from Allegro.cc