Sign in to follow this  
hkBattousai

Simple question - Both 'break' and 'return' in a function

Recommended Posts

bool CMessage::DoesAnyGroupMatch(bool * bUserGroup, bool * bMessageTargets, int nNumGroups)
{
	for (int i=0; i<nNumGroups; i++)
		if (bUserGroup[i] & bMessageTargets[i])
			{
				return true;
				break;
			}
	return false;
}

Do I need to use that 'break' statement in the 'if' block?

Share this post


Link to post
Share on other sites
Quote:
Original post by Battousai
*** Source Snippet Removed ***

Do I need to use that 'break' statement in the 'if' block?


I see some potential issues in your code.

1)You are using pointers without checking if they are valid so hopefully that won't blow up in your face. Or consider using a const std::vector<bool> & instead of bool *. Then you will not even need to pass nNumGroups.
2)You are using & with booleans, perhaps you meant &&?

And just a personal pet peeve: you really don't need to put things like C, b, and n before variable names/class types as the type can be determined easily with modern IDEs and by examining the context. Again this is personal preference.

Share this post


Link to post
Share on other sites
Quote:
Original post by nobodynews
I see some potential issues in your code.

1)You are using pointers without checking if they are valid so hopefully that won't blow up in your face. Or consider using a const std::vector<bool> & instead of bool *. Then you will not even need to pass nNumGroups.
2)You are using & with booleans, perhaps you meant &&?

And just a personal pet peeve: you really don't need to put things like C, b, and n before variable names/class types as the type can be determined easily with modern IDEs and by examining the context. Again this is personal preference.


Thank you all for your replies.

I will learn vectors one day, I promise :) It is in my todo list.
About & mark, I thought & was used for binary boolean operations and && for logical comparisons. Do I know something wrong?

Share this post


Link to post
Share on other sites
Yes, & is for bitwise and, and && is for logical (or Boolean) and. I think the question is, why would you use bitwise when comparing two booleans.

Bitwise is usually only used to test if certain bits of an integer (or other whole number types) are set, Logical-and simply tests if both sides of the operator are non-zero (i.e. both bools are set to true).

Unless you're doing strange bit flags using booleans for some reason, you most likely want &&.

Share this post


Link to post
Share on other sites
Quote:
Original post by dashurc
Unless you're doing strange bit flags using booleans for some reason, you most likely want &&.

Using & saves one conditional jump in the generated assembly.

Share this post


Link to post
Share on other sites
Quote:
Original post by DevFred
Quote:
Original post by dashurc
Unless you're doing strange bit flags using booleans for some reason, you most likely want &&.

Using & saves one conditional jump in the generated assembly.


Whilst one conditional jump is probably not worth optimizing for most people, if I recall correctly, the functional difference, in a boolean context, is that & evaluates both arguments but && will short circuit (i.e. in false & f(), f will be called, but in false && f(), f() won't be called).

I may be wrong.

Share this post


Link to post
Share on other sites
Quote:
Original post by TheUnbeliever
Quote:
Original post by DevFred
Quote:
Original post by dashurc
Unless you're doing strange bit flags using booleans for some reason, you most likely want &&.

Using & saves one conditional jump in the generated assembly.


Whilst one conditional jump is probably not worth optimizing for most people, if I recall correctly, the functional difference, in a boolean context, is that & evaluates both arguments but && will short circuit (i.e. in false & f(), f will be called, but in false && f(), f() won't be called).

I may be wrong.


Yes that's correct.
Whether or not the extra jump makes any difference depends on the processor and the circumstances of the call - it's possible for the jump to be faster than not in some cases.

Share this post


Link to post
Share on other sites
Quote:
Original post by nobodynews
1)You are using pointers without checking if they are valid so hopefully that won't blow up in your face. Or consider using a const std::vector<bool> & instead of bool *. Then you will not even need to pass nNumGroups.
Unless you are exporting the method from a DLL, in which case you need to be absolutely certain the vector implementations match across the client app and your DLL - particularly with vector<bool> which specialises down to a bitvector on most (ifnotall?) Std C++ lib implementations.

Quote:
Original post by nobodynewsAnd just a personal pet peeve: you really don't need to put things like C, b, and n before variable names/class types as the type can be determined easily with modern IDEs and by examining the context. Again this is personal preference.
Totally agree.

additionally, chucking in a couple of asserts wouldn't hurt your style: e.g.
assert(bUserGroup != NULL);
assert(bMessageTargets != NULL);
assert(nNumGroups >= 0);


one last thing:
Quote:
Original post by DevFredUsing & saves one conditional jump in the generated assembly.
this is entirely personal opinion, but I absolutely hate optimisations like this. For me, the value of code clarity outweighs code speed (within reason obviously, and obviously there are exceptions to the rule, but as far as I'm concerned, these should only ever arise in mission critical/real-time control systems code)

Share this post


Link to post
Share on other sites
Quote:
Original post by DevFred
Quote:
Original post by dashurc
Unless you're doing strange bit flags using booleans for some reason, you most likely want &&.

Using & saves one conditional jump in the generated assembly.


Er, I would be very surprised if the compiler couldn't optimize this. & and && are equivalent for single-bit values, which 'bool' (being a real type in C++) should be. However, the function name talks about "matching", which isn't a boolean-and; it's a comparison.

To the OP, using vectors here is quite easy:


// Notice the grammar used in the function name.
// Write the name as the property you're testing for, not the question you would
// ask. It tends to read better in the calling code this way.
bool Message::AnyGroupMatches(const std::vector<bool>& users, const std::vector<bool>& messageTargets)
{
// The vectors already know how big they are, so you don't have to pass
// that information along.
assert (users.size() == messageTargets.size());
int size = user.size();
for (int i=0; i < size; ++i)
// As suggested above: are you sure you didn't mean == ?
if (users[i] && messageTargets[i])
return true;
return false;
}

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