If then assignment

[billfor]

I am preparing for a code review. Our code reviews are very thorough. I need an oppinion on
something. Should this first code block labeled (Before:) be rewritten into the second code
block labeled After: ???

In general I think we try to avoid more efficent and less lines of code. So I don't know wether
the After code block is better?? Also, is it okay to assign a value to the variable Flag at line
5 in the after code block. The value is being assigned in a if statement, and I do not know if this
is okay.

statement.


Before :

1 bool SomeClass::SomeMethod(___)
2 {
3 boolean Flag=false;
4 Flag = myObeject.myMethod( inParam);
5
6 if(!Flag)
7 {
8 FunctionGetSomeData(pIndex,Dvar),
9 FunctionWriteData(Dvar);
10 }
11
12 /* about 25 more lines of code below this */
.
37 return Flag;
38}

After :

1 bool SomeClass::SomeMethod(___)
2 {
3 boolean Flag;
4
5 if(!(Flag = myObeject.myMethod( inParam)))
6 {
7 FunctionGetSomeData(pIndex,Dvar),
8 FunctionWriteData(Dvar);
9 }
10 /* about 25 more lines of code below this */
.
35 return flag
36 }

[willchop]

You might consider not even bothering with the flag
variable.

bool SomeClass::SomeMethod(___)
{
if (!myObeject.myMethod( inParam))
{
FunctionGetSomeData(pIndex,Dvar),
FunctionWriteData(Dvar);
return false;
}

return true;
}


regards, willchop

[jfaust]

The first one is better, but you might as well put it on the same line:



1 bool SomeClass::SomeMethod(___)
2 {
3 boolean Flag = myObeject.myMethod( inParam);
4
5 if(!Flag)
6 {
7 FunctionGetSomeData(pIndex, Dvar),
8 FunctionWriteData(Dvar);
9 }
11
12 /* about 25 more lines of code below this */
.
37 return Flag;
38}


Your second example leaves the Flag variable uninitialized, which should be avoided. Also, the name Flag does not mean anything.

Jeff

[villemos]

Originally posted by billfor
1 bool SomeClass::SomeMethod(___)
2 {
3 boolean Flag;
4
5 if(!(Flag = myObeject.myMethod( inParam)))
6 {
7 FunctionGetSomeData(pIndex,Dvar),
8 FunctionWriteData(Dvar);
9 }
10 /* about 25 more lines of code below this */
.
35 return flag
36 }

Or, if the flag is used later in the "about 25 more lines of code below this", use the copy constructor. Else don't bother about flag;

1 bool SomeClass::SomeMethod(___)
2 {
3 boolean Flag(myObeject.myMethod(inParam));
4
5 if(Flag == false)
6 {
7 FunctionGetSomeData(pIndex,Dvar);
8 FunctionWriteData(Dvar);
9 }
10 /* about 25 more lines of code below this */

Has the benefit of not initailising flag first and then assigning.

By the way; I prefere always to compare for example "flag" to something (i.e. flag == false instead of !flag). Any oppinions anyone?

villemos.

[jfaust]

I prefer "if( flag )", for absolutely no reason. I just think it reads better. "If OK then ..." sounds better than "If OK is true then". It's somewhat redundant.

Jeff

[villemos]

Fair enough :D

Anyone with an efficiency/safety reason?

villemos

[PaulWendt]

I want all of my if conditionals to be boolean expressions. If the
variable being tested returns a bool, then I will use only that.
If I'm comparing a pointer for nullness, however, I will do an
explicit comparison against 0 ... which may seem like overkill, but
I feel things are more consistent that way.


bool flag = true;

if (flag)
{
// do whatever
}


SomeClass* p = dynamic_cast<SomeClass*>(somePointer);
if (p != 0) // instead of if (p)
{
// work with p here
}



--Paul

[willchop]

The alternative example I posted earlier may be more efficient
than the others, although I'm not sure. The other examples
are well enough, but do they introduce an additional assignment
(the flag variable) - or is that assignment normally optimized
by the compiler? I would think that in my example the compiler
would create a temporary return value whose scope would cease
immediately after the boolean evaluation. But if a "real" variable
is present to hold the return value, does the temporary still get
created and then get copied to the "real" flag variable? Are these
details compiler specific?

Anyone have some insight on this?

regards, willchop

[vinodp]

----------------------------Reply to------------------
1 bool SomeClass::SomeMethod(___)
2 {
3 boolean Flag(myObeject.myMethod(inParam));
4
5 if(Flag == false)
6 {
7 FunctionGetSomeData(pIndex,Dvar);
8 FunctionWriteData(Dvar);
9 }
10 /* about 25 more lines of code below this */

Has the benefit of not initailising flag first and then assigning.

By the way; I prefere always to compare for example "flag" to something (i.e. flag == false instead of !flag). Any oppinions anyone?
---------------------------------------------------------------
First of all boolean is not a standard C++ data type (see standard 3.9.1) Its MS extension

so for boolean its always good to explicitaly check with false value bcoz boolean may contain value other than true or false.

In case of bool i think it doesn't matter as C++ standard says that check 3.9.1 point 6
"Values of type bool are either true or false"

Vinod

[Axter]

I agree with willchop, and that you should remove the flag if you're worried about efficiency.
I also agree with one of the other experts in that you should do a test for true, instead of false.
Test for true are easier to read, and less prone to mistakes then test for false.

With that in mind, I recommend the following code:


bool SomeClass::SomeMethod(___)
{
if (myObeject.myMethod( inParam)) return true;
FunctionGetSomeData(pIndex,Dvar),
FunctionWriteData(Dvar);
return false;
}


I think the above code is more compact, and easier to read.

[vinodp]

code:--------------------------------------------------------------------------------
bool SomeClass::SomeMethod(___)
{
if (myObeject.myMethod( inParam)) return true;
FunctionGetSomeData(pIndex,Dvar),
FunctionWriteData(Dvar);
return false;
}
--------------------------------------------------------------------------------
Axter, i think its always good to have single return point in function
bcoz its easy to maintain the function & less chances of resource leak
So from maintainace point of view i am not agree with ur implementation

Vinod

[willchop]

If readability is a concern I would additionally suggest
returning an enumerated status value (eGood, eBad)
instead of a boolean value. Often there is confusion and
conflicts between functions and the meaning of their
boolean result

This code is both effiicent and readable:

EStatus SomeClass::SomeMethod(___)
{
if (myObeject.myMethod( inParam) == eGood)
return eGood;

FunctionGetSomeData(pIndex,Dvar),
FunctionWriteData(Dvar);
return eBad;
}

regards, willchop

[willchop]

Originally posted by vinodp
Axter, i think its always good to have single return point in function
bcoz its easy to maintain the function & less chances of resource leak
So from maintainace point of view i am not agree with ur implementation


vinodp,
I partially agree with you. Although, sometimes keeping the
return status in scope until the end of a function can also be
a maintenance problem (status flags tend to get recklessly
re-used by programmers who aren't familiar with the code).
Also, keeping track of which resources to release at the end
of the function according to the state the function left them in
at the point of the error/end-of-control can also be
maintenance/readiblity/operational problem.

regards, willchop

[Axter]

Axter, i think its always good to have single return point in function
bcoz its easy to maintain the function & less chances of resource leak
So from maintainace point of view i am not agree with ur implementation

I disagree. It depends on the code. If you have a large amount of code in a function, then I agree, that it's better to have one exit point.
But when you have a small function, the function ends up looking more complicated then it has to be when it's forced to have only one exit.
When possible, it's best to keep it simple. Keeping it simple makes it more readable and easier to maintain.

[villemos]

I think billfor needs to clarify something;
Does the 25 lines of code which comes after the initial Flag check use Flag? If not, then I agree with Axter and willchop's suggestions. Else the suggestions may need some more work. Anyway, chances are, as it is a boolean flag, that you no matter what can leave it out completly, i.e. do as Axter and willchop suggest below in the 25 lines of code as well if Flag is used.

I'm not sure about the single exit point, even in short functions, and most functions I try to keep short and to the point.

I prefere functions to return a default value, i.e. the most "normal" condition. Whats normal depends on the function and please I don't wan't to get into a long filosophical discussion of "normallity". I know its subjective. Unormal conditions that should break of the operation returns in the function after handling their condition. I find that else the code tends to become something like

STATUS
A::function doStuff()
{
STATUS status (GOOD);
// Do Stuff A
if(status)
{
// DoStuff B
if(status)
{
status = GOOD;
}
else
{
status = BAD;
}
}
else
{
status = BAD;
}

// Do Stuff C

return status;
}

And here I have been nice and only included two status checks instead of the 10 that will surely be present somewhere in one of your functions ;) I'm not sure is more readable or maintaniable than

STATUS
A::function doStuff()
{
STATUS status (GOOD);
// Do Stuff A
if (status == BAD)
{
return status ;
}

// Do Stuff B
if (status == BAD)
{
return status;
}

// Do Stuff C

return status;
}

Or in operators; Is (I left out the {} as my company only have a limited budget of brackets)

int
operator==(const Object& obj) const
{
boolean Flag(false);
if (obj.A == this->A)
if (obj.B == this->B)
if (obj.C == this->C)
flag = true;

return Flag;
}

More readable or maintaniable than

int
operator==(const Object& obj) const
{
if (obj.A == this->A)
if (obj.B == this->B)
if (obj.C == this->C)
return true;

return false;
}

But I agree on the principle; Keep it simple and if possible have only one return point. Functions with 10+ if statements is likely overcomplicated anyway.

villemos

[Graham]

Axter, i think its always good to have single return point in function bcoz its easy to maintain the function & less chances of resource leak
So from maintainace point of view i am not agree with ur implementation
Given that exceptions exist, it is (nearly) impossible to state with certainty that you only have a single return point in your code. You may only have one return statement, but the function could exit from any number of places. Having a single return statement categorically does not reduce the chances of resource leaks. RTTI does that.

As for the OP, I would be tempted to lose the flag (unless it's used elsewhere), except for the logical "not" in there. I don't like "nots": they're easy to miss and they screw up your brain trying to work out whether the expression is true or not (no pun intended). So I'd probably recast the original code as

bool do_the_extra_stuff = !myObject.method();

if (do_the_extra_stuff)
{
// do the extra stuff
}

The "!" can still be missed, but at least the if test now says something meaningful.

Basically, the bit between the parentheses after the "if" should say something meaningful. If necessary, invent a variable with a name that decribes the condition (positively) and let the optimiser get rid of it in production code.

[Graham]

Originally posted by villemos
Is (I left out the {} as my company only have a limited budget of brackets)

int
operator==(const Object& obj) const
{
boolean Flag(false);
if (obj.A == this->A)
if (obj.B == this->B)
if (obj.C == this->C)
flag = true;

return Flag;
}

More readable or maintaniable than

int
operator==(const Object& obj) const
{
if (obj.A == this->A)
if (obj.B == this->B)
if (obj.C == this->C)
return true;

return false;
}


int operator==() ???

bool operator==(const Object& rhs) const
{
return A == rhs.A && B == rhs.B && C == rhs.C;
}

reads OK to me.

[villemos]

int operator==() ???

int operator==() works fine as bool->int, int->bool conversions are done implictly according to the standard rules. Maybe not nice though...I think thats another discussion.

Graham, your example indeed reads fine, and is the way to do the operator, but the discussion was not how to do operator==() but when to return. The operator was just an example.

villemos.

[Musaran]

Originally posted by jfaust
I prefer "if( flag )", for absolutely no reason. I just think it reads better. "If OK then ..." sounds better than "If OK is true then". It's somewhat redundant.Jeff That is a reason, and a good one.
Such boolean variables should have explicit names that sound good in conditionals:if(isFinished) ....

Originally posted by PaulWendt
I want all of my if conditionals to be boolean expressions. If the variable being tested returns a bool, then I will use only that.
If I'm comparing a pointer for nullness, however, I will do an explicit comparison against 0 ... which may seem like overkill, but I feel things are more consistent that way.I think pretty mich the same, with some variations:
-I prefer the infamous NULL over 0, as I feel it's more explicit about what is done. 0 has many other uses than null pointer.
-I skip explict true/false comparison not only with bool type, but with values that have boolean meaning.
Reason: comparison with true is dangerous !if(isdigit(c)==true) ...; //this...
if(isdigit(c)==1) ...; //...means this, and is buggedtrue gets promoted to what isdigit returns: int.
But isdigit can return other than 1 as meaning true...


Now for the original question, here is what I would do:bool SomeClass::SomeMethod(___){
bool noExtra= myObeject.myMethod(inParam);
if(!noExtra){
FunctionGetSomeData(pIndex,Dvar);
FunctionWriteData(Dvar);
}
/* about 25 more lines of code below this */
return noExtra;
}
or:
bool SomeClass::SomeMethod(___){
bool noExtra= myObeject.myMethod(inParam);
switch(noExtra){
case false:
FunctionGetSomeData(pIndex,Dvar);
FunctionWriteData(Dvar);
default:
/* about 25 more lines of code below this */
}
return noExtra;
}
The original code does not return in the if !

To mention:bool noExtra= myObeject.myMethod(inParam);
bool noExtra(myObeject.myMethod(inParam));These are equivalently an initialisation, even with class elaborate types.
The construtor syntax doesn't suit a fundamental type.

[Graham]

Originally posted by Musaran
That is a reason, and a good one.
Such boolean variables should have explicit names that sound good in conditionals:if(isFinished) ....

<SNIP>

Now for the original question, here is what I would do:bool SomeClass::SomeMethod(___){
bool noExtra= myObeject.myMethod(inParam);
if(!noExtra){
FunctionGetSomeData(pIndex,Dvar);
FunctionWriteData(Dvar);
}
/* about 25 more lines of code below this */
return noExtra;
}

<SNIP>

I have to disagree slightly here: your if test contains a double negative: "not noExtra". These can be notoriously hard to read. Think about it: the statement is saying "if it's not true that I should not do extra processing.....". Compare that with:

bool DoExtra = ! myObject.myMethod();
if (DoExtra)
{ .....

Now the if test says "if I should do extra processing...".

As a general rule, just about any variable is better named for what it is, rather than what it isn't. If you find yourself regularly testing the NOT of a variable, I would suggest inverting the name and intialisation of the variable.

On the side issue of "if (ptr /* == NULL|0 */)" I look at it this way: in ANY conditional, there is an implied "!= 0". So, "if (p)" is implicity treated as "if (p !=0)". If you write "if (p != 0)", that becomes implicitly "if ((p != 0) != 0)", since the comparison operators return 0 or 1 as their values.

(complete aside here: it's possible to intialise a unit matrix with that fact:

double mat[N][N];
for (i = 0; i < N; ++i)
for (j = 0; j < N, ++j)
mat[ i ][ j ] = i == j;

Not that I would ever advocate such a thing, of course :) )

Anyway, that implied "!= 0" is enough to make me stop and think about whether adding "!= 0" really improves the readability. Sometimes it does, most times it doesn't.

[Musaran]

Originally posted by Graham
I have to disagree slightly here: your if test contains a double negative: "not noExtra". These can be notoriously hard to read.Yes, of course.
But the boolean is used reversed between the condition and return, so you have to negate it somewhere.
I just found the grouping of 2 negations '!no.." to be understandable enough.

One could also skip the variable and call the function twice.
It looses the explicitness of the variable name.
It spares the trouble of an extra variable and it's name choosing.
It avoids the long distance between variable creation and use.
It requires an extra function call (I don't like that).

in ANY conditional, there is an implied "!= 0".In C++, it is an implied single cast to boolean in fact.
You can spy this by overloading "!=", "bool" and "int" operators with a cout in a class.
The effect is mostly the same.

if(p ); //I use that in low-level code
if(p!=NULL); //I use that when things could be confusing
if(p!=0 ); //I don't like this hybrid

class Thing{
virtual drop()= NULL; //I also prefer this
};
Not very big matters in fact...

[jfaust]

if(p!=NULL); //I use that when things could be confusing
if(p!=0 ); //I don't like this hybrid


Hybrid? 0, at least, is part of C++. 'NULL' is the hybrid. There is no doubt what 0 is. NULL can be and is defined differently in different evironments.

Jeff

[Graham]

Originally posted by Musaran
Yes, of course.
But the boolean is used reversed between the condition and return, so you have to negate it somewhere. Which is why I negated it at initialisation and gave the variable a positive name.
I just found the grouping of 2 negations '!no.." to be understandable enough. Fair enough: I tend to find them less readable.In C++, it is an implied single cast to boolean in fact. ... and it determines which value of the booean type to cast to by comparing it to zero.
if(p ); //I use that in low-level code
if(p!=NULL); //I use that when things could be confusing
if(p!=0 ); //I don't like this hybridComparing a pointer to (literal) zero is not a hybrid: it is carefully defined in the standard. What is not defined in the standard is comparing a pointer to NULL.

class Thing{
virtual drop()= NULL; //I also prefer this
};Assigning NULL to a virtual function is not defined, either - you're relying on NULL being defined as "0" with no other qualification.
Not very big matters in fact... No, not big matters, and certainly not things I would want to fall out over. But, after many years of taking over other people's code, sometimes its the little things that irritate the most.....

[Musaran]

Graham
... and it determines which value of the booean type to cast to by comparing it to zero.Except if you have overloaded "bool" cast.
You could have a numerical class considered "true", even though it's arithmetic value would be 0 (not to be recommended).
Classes such as "cin" have no arithmetic value, but have a boolean one.


jfaust
Hybrid? 0, at least, is part of C++. 'NULL' is the hybrid. There is no doubt what 0 is. NULL can be and is defined differently in different evironments.
Graham
Comparing a pointer to (literal) zero is not a hybrid: it is carefully defined in the standard. What is not defined in the standard is comparing a pointer to NULL.
I have not been clear enough.
What I know is wrong with NULL:
-It is a macro, and macro are dangerous "old machinery".
-It is C, not directly C++.
-It has to come from some header.
-It is different on different systems ( typically 0 or ((void*)0) ).

What I think is good with NULL:
It is well-defined as meaning "null pointer", since very long, and only a fool would do otherwise (I know there are fools).

What to use as a null-pointer/value is a very difficult problem, and C++ solved it by making 0 a magical number that can safely and automatically be turned into any of the fundamental types, including pointers, where it means appropriate null-pointer.
Do you remember something along the lines of "magical numbers are bad" ?

What are the various meanings of "0" here ?int func(const char *s){
int v= 0;
if(s==0) return 0;
for( ; s[0] != 0 ; ++s){
v<<= 1 ;
v|= s[0] - '0';
}
return v;
}When I must specify null-pointer, I feel NULL is much more appropriate and explicit than 0.
When I must specify null-char , I feel '\0' is much more appropriate and explicit than 0.
When I must specify error return, I feel enum is much more appropriate and explicit than 0 (or else).
I use 0 only as number zero, in computations and/or indices.

But I am just a beginner, you have coded longer than me, and if you have found NULL to be problematic, I can acknowledge that.

Graham
Assigning NULL to a virtual function is not defined, either - you're relying on NULL being defined as "0" with no other qualification.Yes.
There is obviously a pointer at work underneath, but I will stick to 0 in this case, since it is not likely to be misunderstood.

[Graham]

Originally posted by Musaran
Except if you have overloaded "bool" cast.
You could have a numerical class considered "true", even though it's arithmetic value would be 0 (not to be recommended).
Classes such as "cin" have no arithmetic value, but have a boolean one.
In which case, it's not an implied cast anymore, is it?
What to use as a null-pointer/value is a very difficult problem, and C++ solved it by making 0 a magical number that can safely and automatically be turned into any of the fundamental types, including pointers, where it means appropriate null-pointer.
Do you remember something along the lines of "magical numbers are bad" ?
Not in this instance. The proscription against "magic numbers" is quite different.
What are the various meanings of "0" here ?When I must specify null-pointer, I feel NULL is much more appropriate and explicit than 0.
When I must specify null-char , I feel '\0' is much more appropriate and explicit than 0.
When I must specify error return, I feel enum is much more appropriate and explicit than 0 (or else).
I use 0 only as number zero, in computations and/or indices.
Fair enough. I'm not trying to bully you into never using NULL. I happen to think that 0 is perfectly legible in all cases. Context is all. In English, there are over a dozen meanings for the sound "so" (or "sew"). You can generally figure out which one a person means from context. For the same reason, I see no problem with zero having slightly different meanings in different places.
One thing, though. If you happen to compile your C++ program in an environment where NULL is defined as ((void*)0) [or something similar], be prepared to be overwhelmed with error messages.

[cvk]

Hi.
I have same issue to resolve in near future.
and i have some ideas about design.

1.Each object must protect itself.
any other foreign functions can't manage lock status.
They process returncode or catch exception
if object is locked.


2. object needs for more types of locks such us:

strong lock - placed by constructors, destructor and other critical sections
any functions don't have access to object
read lock - any functions don't change object but can read it at the same time
write lock - one of functions can change object , other functions don't have access.

typedef lock_F 0x00; // object dont have any locks
typedef lock_S 0x80 ;// strong lock
typedef lock_R 0x01;// ReadLock
typedef lock_W 0x04; //Write lock

class Object
{
unsigned int readcounter; // number of reads at this time
unsigned char lock;
//some data
//=====================================
private:

int PlaceLock(char LockLevel)
{
if((lock>LockLevel) || ( lock>lock_R) ) //
{ // current locking less then exist lock
// can't place lock( throw exception or return error)
};
if(lock==lock_R) //
{
readcounter=readcoutner++;
return true;
}
lock=LockLevel; // place lock default
return true;
};
//=================================
int FreeLock(void)
{
if(lock==lock_R)
{
if(readcounter>=1) //anybody isn't reading object
{
lock=lock_F; // free object
return true;
}
else //somebody is reading object yet
{
readcounter=readcounter--;
return false;
}
}
else // release object

{
lock=lock_F ;
readcounter=1;
};
return true ;
}
//=================================
public:
// some constructors

int ReadData(........)
{
if(PlaceLock(lock_R)
{
//some actions with data
}
else
{
// Object isn't locked, return error or throw exception
}
FreeLock(); // free Object
}
..................

}

I want to show my idea.
Please correct me if i'm wrong.

[cvk]

my code has mistake

int PlaceLock(char LockLevel)
{
if((lock>LockLevel) || ( lock>lock_R) ) // mistake !!!!!!!!!
// because any functions can change object when other
//functions is reading it
// must be a

if(| ( lock>lock_R) || (lock!=lock_F))

{ // current locking less then exist lock
// can't place lock( throw exception or return error)
};
if(lock==lock_R) //
{
readcounter=readcoutner++;
return true;
}
lock=LockLevel; // place lock default
return true;
};

[Musaran]

Originally posted by Graham
In which case, it's not an implied cast anymore, is it? Are we talking about the same thing ?
I am talking about the fact that C++ conditional structures (if for while ?:) implicitly do a (bool) cast on their "operand".
As can be spied with#include <iostream>
using namespace std;

struct test{
operator bool(){
cout<<"Converting to bool"<<endl;
return false;
}
}t;

int main(){
if(t);
while(t);
for(;t;);
do{}while(t);
t?1:0;
return 0;
}

One thing, though. If you happen to compile your C++ program in an environment where NULL is defined as ((void*)0) [or something similar], be prepared to be overwhelmed with error messages. I somehow believed ((void*)0) was fine as null for any pointer... It's not.
I remember now... Scott Meyers, "Effective C++", rule 25. It explains why null pointers are so tough. I think I better re-read the whole book.

cvk
You are making a run-time, multi-thread, multi-level const.
But... does it belong here ?

[Graham]

Originally posted by Musaran
Are we talking about the same thing ?

Probably not.

I'm saying that <T>::operator bool() is a user-defined conversion (UDC), and that a UDC is not an implied (compiler-generated) conversion (e.g. int to double, int to bool, etc.). The original point -whether to explicitly put "!= 0" (or "!= NULL") on a test - involves such a compiler-generated (implied) conversion, since pointers are fundamental types. My point was that, when the compiler converts a pointer or an int to bool then an implicit "!= 0" underlies the implementation of that conversion. I'll admit that the curiously recursive nature of "i != 0" is a hangover from C and predates the type "bool" (and is, I suppose, removed by it), but old habits die hard, and I still mentally add that "!= 0" to something like "if (i)".

Still, it's a pretty trivial point when it comes down to it. I'm trying to cure myself of the old habit of "if (ptr)" because I just prefer the look of "if (ptr != 0)" (and, no, I don't like NULL there). Of course, I'd rather arrange things so that I don't need the test at all, but that's another thread.....

[Musaran]

Yes, C++ defines 0 or null as false, anything else as true.
But I got bad experiences from "obvious" and "trivial" in C++.
There are most often subtle situations where the exact meaning has deep impact on the way the program works or, well... doesn't.

This one surprised me:void* f(int i){ return (void*)i; }

int main(){ return (bool)f(0); }
What is the exit code ? Obviously 0... But it could be 1. Wich by the way is not standard.

I too try to avoid tests whenever possible. Bad experiences again... multiple branches to test.

And on second though "hasExtra" is indeed better than "noExtra".