CODEWITHSUNDEEP
c++c
user1534664
user1534664

Reputation: 3418

C++ SDL Breaking out of while loop

I've been messing around with C++ SDL for a few days now and I've come across an interesting problem.

SDL_Event event1;
while(SDL_WaitEvent(&event1))
{
    for(size_t i = 0; i < MainMenuOptions.size();i++)
    {
        if(event1.button.x > MainMenuOptions.at(i).GetX() && event1.button.x < (MainMenuOptions.at(i).GetX() + MainMenuOptions.at(i).GetWidth())  
        && event1.button.y > MainMenuOptions.at(i).GetY()  && event1.button.y < (MainMenuOptions.at(i).GetY() + MainMenuOptions.at(i).GetHeight()))
        {
            break;
        }
    }
}

When I use break in the for loop its going to break out of the for loop instead of the while loop. How would I break out the while loop instead without using the goto statement? (the goto statement is bad programming, I heard)

Upvotes: 2

Views: 770

Answers (5)

Bartek Banachewicz
Bartek Banachewicz

Reputation: 39390

There's another answer to that, and I think I should say it before everyone will downvote me. Using a variable is certainly a "good" way to do it. However, the creating additional variable just to jump out of the loop seems a bit of overkill, right?

So yes, this time goto is perfect solution. It's perfectly clear what you are doing with it, you are not using another variable and the code remains short, maintainable and readable.

The statement goto is bad practice is mostly a remnant of the BASIC times, when it was quite the only way of changing code flow. However, now we "know better", and saying that the goto or any other construction is bad, just doesn't cut it. It can be bad for one particular problem you are trying to solve with it (and it's the case with most of the problems that people try to solve with goto). However, given the right circumstances (like here) it's OK. I don't want to start a debate here, of course. Goto is like a very powerful tool (sledgehammer, for example). It has its uses and you can't say a tool is totally bad; it's the user using it in the wrong way.

Upvotes: 7

Jerry Coffin
Jerry Coffin

Reputation: 490328

First point: IMO, you're trying to wrap too much up into a single place, and ending up with something that's fairly difficult to understand -- somebody has to read through that entire long set of comparisons before they can understand any of what this is supposed to accomplish at all.

Second point: using an explicit loop to iterate over a standard collection is usually a mistake -- and this is no exception. The standard library already has an algorithm to accomplish the same basic thing as your loop. It's better to use that than write it again yourself.

template <class T>
bool in_range(T a, T b, T c) { 
    return (a > b) && (a < b+c);
}

class in_rect {
   point p;
public:
   in_rect(point const &p) : p(p) {}

   // Not sure of the type of objects in MainMenuOptions, so just T for now.
   // 
   bool operator()(T const &m) {
       return in_range(p.x, m.GetX(), m.GetWidth()) 
           && in_range(p.y, m.GetY(), m.GetHeight());
   }
};

SDL_Event event1;

while (SDL_WaitEvent(&event1))
    if (std::any_of(MainMenuOptions.begin(), MainMenuOptions.end(),
                    in_rect(event1.button))      
        break;

Once we fix the other problems, there's simply no longer any need (or even use) for the goto. We haven't taken any steps explicitly intended to remove it, but when the other problems have been fixed (especially, replacing the loop with an appropriate algorithm), the use for it has disappeared.

I suppose I should preemptively comment on the increase in the total number of lines of code: yes, there are more lines of code. What of it? If we really wanted to, we could use the same basic approach, but instead of defining in_rect and in_range, we'd basically just take the condition from the original if statement and stuff it into a lambda. While I'm very happy that lambdas have been added to C++, in this case I'm not excited about using it. It would get rid of the goto, but in general the code would be almost as unreadable as it started out.

Simply put, the number of lines isn't a good way to measure much of anything.

Upvotes: 4

sbi
sbi

Reputation: 224119

The common solution is to put this stuff into its own function and return from that: 

inline SDL_Event do_it()
{
    SDL_Event event;
    while(SDL_WaitEvent(&event))
        for(std::size_t i = 0; i < MainMenuOptions.size(); ++i)
            if(/*...*/)
                return event;
    return event; // or whatever else suits, I know too little about your code
}

Upvotes: 11

Christian Ammer
Christian Ammer

Reputation: 7542

A solution without additional variable and goto:

while(SDL_WaitEvent(&event1))
{
    size_t i;
    for(i = 0; i < MainMenuOptions.size();i++)
    {
        if(/* ... */)
        {
            break;
        }
    }
    if (i < MainMenuOptions.size())
        break;
}

Upvotes: -1

Timbo
Timbo

Reputation: 28060

Use a variable to indicate the need to exit:

bool exit_program = false;

while( !exit_program && SDL_WaitEvent(&event1) )
{
    for( /* ... */ )
    {
        exit_program = true;
    }
}

Upvotes: 4

Related Questions