Sign in to follow this  
cozzie

Breaking out of for loop

Recommended Posts

cozzie    5029

Hi,

I have perhaps a relatively simple question about breaking out of a for loop.
​My current code does the following:

- loop through a std::vector of input contexts
​- if the requested button can be mapped as an action or state in a context, the for loop stops.
​So basically I map the button to an action, in the 1st context the loop runs into and then stop (a button can only lead to 1 action, in the 1st context it finds a mapping). The contexts are sorted on priority [0] being the highest priority.

The code:

void CInputManager::SetRawButtonState(const RAW_BUTTON pButton, const bool pState, const bool pPrevState)
{
	for(auto &context : mContexts)
	{
		if(context.IsActive())
		{
			if(context.MapButtonToAction(pButton)) break;
			else
			{
				if(context.MapButtonToState(pButton, pState)) break;
			}
		}
	}
}

Now, what I want to do is add another check:
​- I only want to try to map button to action in a certain condition (pState = true, pPrevState = false)

After trying to add that to the code, my mind gets confused, is it correct how I tried this, or is the theory on the 'break' no langer valid (because of if statement within if statement). 

Back to the basics: I think that the break simply takes me out of the for loop. If so, all is good.
​Can someone confirm this or tell me where my mind confuses me? :)

The attempt:

void CInputManager::SetRawButtonState(const RAW_BUTTON pButton, const bool pState, const bool pPrevState)
{
	for(auto &context : mContexts)
	{
		if(context.IsActive())
		{
			bool actionDone = false;
			if(pState && !pPrevState)
			{
				if(context.MapButtonToAction(pButton)) actionDone = true;
			}
			if(actionDone) break;
			else
			{
				if(context.MapButtonToState(pButton, pState)) break;
			}
		}
	}
}

Share this post


Link to post
Share on other sites
frob    44962

Back to the basics: I think that the break simply takes me out of the for loop. If so, all is good.
 

It looks like you're good.

A break statement will exit any of these: for loops, while loops, do/while loops, and switch blocks.  The funner issues come when you need to exit multiple levels of loops.

Break statements don't care about the if statements or the { } block delimiters, they'll pass right over them.

Since actionDone is within the for loop, that code can use break directly instead of setting the flag then testing the flag.

Share this post


Link to post
Share on other sites
cozzie    5029
Thanks both.

@Apoch: I'm actually working on the implementation of my input wrapper based on your article from 2011 (by head) :)

Share this post


Link to post
Share on other sites
nobodynews    3126
void CInputManager::SetRawButtonState(const RAW_BUTTON pButton, const bool pState, const bool pPrevState)
{
    for(auto &context : mContexts)
    {
        if(context.IsActive())
        {
            if(pState && !pPrevState)
            {
                if(context.MapButtonToAction(pButton)) break;
            }
            if(context.MapButtonToState(pButton, pState)) break;
        }
    }
}

FWIW I believe this should perform the same and I find it to be simpler to read.

 

edit: and if you want to compress it further:

void CInputManager::SetRawButtonState(const RAW_BUTTON pButton, const bool pState, const bool pPrevState)
{
    for(auto &context : mContexts)
    {
        if(context.IsActive())
        {
            if(pState && !pPrevState && context.MapButtonToAction(pButton)) break;

            if(context.MapButtonToState(pButton, pState)) break;
        }
    }
}
Edited by nobodynews

Share this post


Link to post
Share on other sites
alvaro    21263
How about this?

void CInputManager::SetRawButtonState(const RAW_BUTTON pButton, const bool pState, const bool pPrevState)
{
    for(auto &context : mContexts)
    {
        if(!context.IsActive()) continue;
        
        if(pState && !pPrevState && context.MapButtonToAction(pButton)) break;
        
        if(context.MapButtonToState(pButton, pState)) break;
    }
}
Edited by �lvaro

Share this post


Link to post
Share on other sites
cozzie    5029

Thanks all.

@Ferrous: that's an option too, indeed. Basically breaking out of the for loop will result in the same as a return (in this specific case), because the function stops after the for loop.

Share this post


Link to post
Share on other sites
frob    44962

If your function isn't abbreviated for your posts, you could also return instead of break.

In many code style guides --- and for general readability --- it is usually best not to do that.

Typically code can have some early-out returns at the beginning, such as empty work or as a safety net for assertions, and then only one return at the end of the body.

When you mix a bunch of returns through the code it makes it harder for future work to make modifications safely.  TODAY it might make sense to return from the loop, but TOMORROW you may want to add some functionality that happens after the loop, but since sometimes the code returns and sometimes not, the code will need to be rewritten so that all the options follow a uniform flow.

Generally it is best to stick with the uniform flow from the beginning.  Code may go through if conditionals, but otherwise should all follow the same paths. 

Share this post


Link to post
Share on other sites
ApochPiQ    23058
There's a delicate balance there, IMO.

I could read your argument as basically advocating for never refactoring anything but just writing it the "best way" up front. I don't think that's actually what you meant, though :-)


Sometimes requirements change, and the understanding of requirements changes frequently. Treat code like a fluid. Don't be scared to modify it when (not if) you encounter a reason to.

If you write code without fear of modifying it, you can set yourself free from the need to get everything perfect on the first pass - and also eliminate a lot of "best practice" rules that purport to make your code better.


The more I program and the more I read other people's code, the less I'm convinced that there is a magical set of rules one can follow that will be less problematic than all others in the long term.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this