views:

447

answers:

6

In Stroustrup's The C++ Programming Language: Special Edition (3rd Ed), Stroustrup writes that the declaration and initialization of variables in the conditionals of control statements is not only allowed, but encouraged. He writes that he encourages it because it reduces the scope of the variables to only the scope that they are required for. So something like this...

if((int i = read(socket)) < 0) {
    // handle error
}
else if(i > 0) {
    // handle input
}
else {
    return true;
}

...is good programming style and practice. The variable i only exists for the block of if statements for which it is needed and then goes out of scope.

However, this feature of the programming language doesn't seem to be supported by g++ (version 4.3.3 Ubuntu specific compile), which is surprising to me. Perhaps I'm just calling g++ with a flag that turns it off (the flags I've called are -g and -Wall). My version of g++ returns the following compile error when compiling with those flags:

socket.cpp:130: error: expected primary-expression before ‘int’
socket.cpp:130: error: expected `)' before ‘int’

On further research I discovered that I didn't seem to be the only one with a compiler that doesn't support this. And there seemed to be some confusion in this question: http://stackoverflow.com/questions/136548/pro-con-initializing-a-variable-in-a-conditional-statement-c as to exactly what syntax was supposedly standard in the language and what compilers compile with it.

So the question is, what compilers support this feature and what flags need to be set for it to compile? Is it an issue of being in certain standards and not in others?

Also, just out of curiosity, do people generally agree with Stroustrup that this is good style? Or is this a situation where the creator of a language gets an idea in his head which is not necessarily supported by the language's community?

+3  A: 

I've run into a similar problem:

The problem seems to be the parentheses around the int declaration. It should work if you can express the assignment and test without them. i.e.

if (int i = read(socket)) {

should work, but that means the test is != 0 which is not what you want.

Ferruccio
+4  A: 

I consider it a good style when used with possibly NULL pointer:

if(CObj* p = GetOptionalValue()) {
   //Do something with p
}

This way whether p is declared, it is a valid pointer. No dangling pointer access danger.

On the other hand at least in VC++ it is the only use supported (i.e. checking whether assignment is true)

EFraim
+6  A: 

It is allowed to declare a variable in the control part of a nested block, but in the case of if and while, the variable must be initialized to a numeric or boolean value that will be interpreted as the condition. It cannot be included in a more complex expression!

In the particular case you show, it doesn't seem you can find a way to comply unfortunately.

I personally think it's good practice to keep the local variables as close as possible to their actual lifetime in the code, even if that sounds shocking when you switch from C to C++ or from Pascal to C++ - we were used to see all the variables at one place. With some habit, you find it more readable, and you don't have to look elsewhere to find the declaration. Moreover, you know that it is not used before that point.


Edit:

That being said, I don't find it a good practice to mix too much in a single statement, and I think it's a shared opinion. If you affect a value to a variable, then use it in another expression, the code will be more readable and less confusing by separating both parts.

So rather than using this:

int i;
if((i = read(socket)) < 0) {
    // handle error
}
else if(i > 0) {
    // handle input
}
else {
    return true;
}

I would prefer that:

int i = read(socket);
if(i < 0) {
    // handle error
}
else if(i > 0) {
    // handle input
}
else {
    return true;
}
RedGlyph
For the second example, don't use "i" as a variable. For anything other than a strictly local variable, make the name meaningful.
David Thornley
+1  A: 

While you can use a declaration as a boolean expression, you cannot place a declaration in the middle of an expression. I cannot help thinking that you are mis-reading what Bjarne is saying.

The technique is useful and desirable mostly for control variables of for loops, but in this instance I believe is ill-advised and does not serve clarity. And of course it does not work! ;)

if( <type> <identifier> = <initialiser> ) // valid, but not that useful IMO

if( (<type> <identifier> = <initialiser>) <operator> <operand> )  // not valid

for( <type> <identifier> = <initialiser>; 
     <expression>; 
     <expression> )  // valid and desirable

In your example you have called a function with side effects in a conditional, which IMO is a bad idea regardless of what you might think about declaring the variable there.

Clifford
Err, wouldn't you put the result `read(socket) < 0` in the variable i by doing this?
RedGlyph
Exactly; that is what I said - it changes the meaning. The point being that it compiles, declaring a variable in a conditional is supported, but not in the middle of an expression. The parentheses place it in the middle of an expression. What is valid is a declaration with an initialisation expression.
Clifford
My bad! Sorry I misread your explanation :)
RedGlyph
At least one group of user is going to read your first two lines, decide that you're wrong and downvote you. It might be worth changing the first line so that it highlights that you're not suggesting this "solves the problem".
Richard Corden
I am not going to loose sleep over a few StackOverflow votes unless they become exchangeable for Air Miles! ;) But point taken.
Clifford
+2  A: 

I use const as much as possible in these situations. Instead of your example, I would do:

const int readResult = read(socket);
if(readResult < 0) {
    // handle error
} 
else if(readResult > 0)
{
    // handle input
} 
else {
    return true;
}

So although the scope isn't contained, it doesn't really matter, since the variable can't be altered.

Bill
I like that. (Now with 15+ chars)
jmucchiello
+1  A: 

Adding to what RedGlyph and Ferruccio said. May be we can do the following to still declare within a conditional statement to limit its use:

if(int x = read(socket)) //x != 0 { if(x < 0) //handle error { } else //do work { } } else //x == 0
{ return true; }

White_Pawn
I like it, I hadn't though of rearranging my if statements that way :)
Daniel Bingham