Style: Declaration in if/while-condition - good or bad?

Discussion in 'C++' started by Martin Ba, Feb 27, 2013.

  1. Martin Ba

    Martin Ba Guest

    C(++) allows for a simple variable declaration+init in the condition of
    an if or while:

    while (T t = x) {
    }

    if (int x = f()) {
    }

    I find this rather useful for things like:

    if (const T* pThing = GetThing()) {
    ...
    } else {
    // Handle No-Thing case
    }


    However, I have been told by a colleague that this is little used and
    would surprise and confuse a lot of developers and therefore it's better
    to use the form:

    const T* pThing = GetThing();
    if (pThing) {
    ...
    } else {
    // Handle No-Thing case
    }


    Opinions?

    cheers,
    Martin
    Martin Ba, Feb 27, 2013
    #1
    1. Advertising

  2. Fine.

    a) for (int i = 0; i < 10; ++i) { //...
    b) void func(T t);
    etc.
    Pavel Volkovskiy, Feb 27, 2013
    #2
    1. Advertising

  3. On 2/27/2013 7:41 AM, Martin Ba wrote:
    > C(++) allows for a simple variable declaration+init in the condition of
    > an if or while:
    >
    > while (T t = x) {
    > }
    >
    > if (int x = f()) {
    > }
    >
    > I find this rather useful for things like:
    >
    > if (const T* pThing = GetThing()) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }
    >
    >
    > However, I have been told by a colleague that this is little used and
    > would surprise and confuse a lot of developers and therefore it's better
    > to use the form:
    >
    > const T* pThing = GetThing();
    > if (pThing) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }
    >
    >
    > Opinions?


    Your colleague's opinion seems rather unfounded. AFAICT it is quite a
    commonplace to have a pointer declared in the 'if' condition to be used
    only when it evaluates in non-null expression. Unless there is a need
    to use 'pThing' after the 'if' statement, there is no reason for
    'pThing' to be declared in the scope outside of the 'if' it has been
    evaluated for. Does this help? I hope so.

    V
    --
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Feb 27, 2013
    #3
  4. Martin Ba <> writes:
    [...]
    > while (T t = x) {
    > }
    >
    > if (int x = f()) {
    > }


    [instead of]

    > const T* pThing = GetThing();
    > if (pThing) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }


    They are not equivalent, because the former restricts the scope of the
    declaration (you would need a pair of braces around the second piece of
    code to get the same effect).

    I find myself using this more and more, often changing my code to make
    these patterns usable. I think you should not worry about potential
    "surprise and confusion", because the code looks to me completely
    unambiguous, and therefore it takes almost no time to get used to.

    -- Alain.
    Alain Ketterlin, Feb 27, 2013
    #4
  5. Martin Ba <> wrote:
    > C(++) allows for a simple variable declaration+init in the condition of an if or while:
    >
    > while (T t = x) {
    > }
    >
    > if (int x = f()) {
    > }
    >
    > I find this rather useful for things like:
    >
    > if (const T* pThing = GetThing()) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }
    >
    >
    > However, I have been told by a colleague that this is little used and
    > would surprise and confuse a lot of developers and therefore it's better to use the form:
    >
    > const T* pThing = GetThing();
    > if (pThing) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }
    >
    >
    > Opinions?


    Regardless whether someone is actually using it or not, it should at least
    be clear what it does. I don't understand how this is confusing. Everyone
    uses it with 'for' loops.

    Personally, I use it from time to time but it has also some drawbacks.

    Pro:
    - Keeps the outer scope clean.
    - Declaration is very close to the point where it is used, especially in
    'else if' conditions.
    Con:
    - Only possible with implicit conversion to bool instead if explicit
    condition (e.g. pThing != NULL).
    - Complex conditions are not possible. E.g: if (pThing &&
    pThing->IsValid())

    Because of that limitations, one can argue that if the condition changes in
    the future, you have to move it to the outer scope anyway. And those
    changes are potentially dangerous (because you increase the scope of a
    variable) and more difficult to review than just changing the condition.

    Tobi
    Tobias Müller, Feb 27, 2013
    #5
  6. Martin Ba

    Balog Pal Guest

    On 2/27/2013 1:41 PM, Martin Ba wrote:
    > I find this rather useful for things like:
    >
    > if (const T* pThing = GetThing()) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }
    >
    >
    > However, I have been told by a colleague that this is little used and
    > would surprise and confuse a lot of developers and therefore it's better
    > to use the form:
    >
    > const T* pThing = GetThing();
    > if (pThing) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }


    The latter form is way inferior. And developers who actually get
    confused by it (after a 5-minute training) are better have their code
    checkin right suspended. And a workplace with abundance of such people
    is better left ASAP.
    Balog Pal, Feb 27, 2013
    #6
  7. Martin Ba

    Balog Pal Guest

    On 2/27/2013 2:40 PM, Tobias Müller wrote:
    > Con:
    > - Only possible with implicit conversion to bool instead if explicit
    > condition (e.g. pThing != NULL).
    > - Complex conditions are not possible. E.g: if (pThing &&
    > pThing->IsValid())


    Yeah, too bad we have restrictions (other one popping up with for that
    we need multiple vars, and only one is supported; I'm generally not
    happy to use tuple as workaround).

    But that is not an argument against use where actually possible.

    > Because of that limitations, one can argue that if the condition changes in
    > the future, you have to move it to the outer scope anyway. And those
    > changes are potentially dangerous (because you increase the scope of a
    > variable) and more difficult to review than just changing the condition.


    That sounds pretty weird. And as a purely fear-based speculative argument.

    Future changes are problems of the future. Covered well by YAGNI, and
    was experience on our inability to correctly predict the future. So
    better drop all speculation and keep the code simplest and easiest to
    read. Now, and ever. For the task that must be done.

    And I don't see why review the change later would be more dangerous or
    time-consuming than doing it right now.

    While leaving variable around will keep an educated reader wondering on
    its purpose.
    Balog Pal, Feb 27, 2013
    #7
  8. Martin Ba

    Stefan Ram Guest

    Balog Pal <> writes:
    >The latter form is way inferior. And developers who actually get
    >confused by it (after a 5-minute training) are better have their code
    >checkin right suspended.


    If it helps the OP to get a kind of an idea of the
    prevailing/general/average opinion: I agree!
    Stefan Ram, Feb 27, 2013
    #8
  9. Balog Palæ–¼ 2013å¹´2月28日星期四UTC+8上åˆ3時30分38秒寫é“:
    > On 2/27/2013 1:41 PM, Martin Ba wrote:
    >
    > > I find this rather useful for things like:

    >
    > >

    >
    > > if (const T* pThing = GetThing()) {

    >
    > > ...

    >
    > > } else {

    >
    > > // Handle No-Thing case

    >
    > > }

    >
    > >

    >
    > >

    >
    > > However, I have been told by a colleague that this is little used and

    >
    > > would surprise and confuse a lot of developers and therefore it's better

    >
    > > to use the form:

    >
    > >

    >
    > > const T* pThing = GetThing();

    >
    > > if (pThing) {

    >
    > > ...

    >
    > > } else {

    >
    > > // Handle No-Thing case

    >
    > > }

    >
    >
    >
    > The latter form is way inferior. And developers who actually get
    >
    > confused by it (after a 5-minute training) are better have their code
    >
    > checkin right suspended. And a workplace with abundance of such people
    >
    > is better left ASAP.


    How about getting 2 to 5 things in different classes to work together
    in a safe way if anything went wrong?
    88888 Dihedral, Feb 28, 2013
    #9
  10. Martin Ba

    Martin Ba Guest

    On 27.02.2013 20:42, Balog Pal wrote:
    > On 2/27/2013 2:40 PM, Tobias Müller wrote:
    >> ...
    >> Because of that limitations, one can argue that if the condition
    >> changes in
    >> the future, you have to move it to the outer scope anyway. And those
    >> changes are potentially dangerous (because you increase the scope of a
    >> variable) and more difficult to review than just changing the condition.

    >
    > That sounds pretty weird. And as a purely fear-based speculative argument.
    >


    It seems fear-based arguments are "common" when discussing C++. Maybe
    because it's such an awe-inspiring language :)
    Martin Ba, Feb 28, 2013
    #10
  11. Martin Ba

    Martin Ba Guest

    On 27.02.2013 15:26, David Brown wrote:
    > On 27/02/13 13:41, Martin Ba wrote:
    >> C(++) allows for a simple variable declaration+init in the condition of
    >> an if or while.....

    >
    >
    > I think the declaration in the condition style is fine for simple and
    > clear cases (and minimises the scope of the variable, which is always
    > good practice) - but don't do it for complex assignments as they can
    > easily be unclear.
    >
    > Note that some C/C++ checkers will flag "if (x = GetThing()) ..." with a
    > warning (note the lack of declaration - this assumes "x" is already
    > defined and in scope) since it looks very much like a typo with "="
    > instead of "==". gcc will flag this when you have a lot of warnings
    > enabled - use "if ((x = GetThing()))" with extra parenthesis to avoid
    > the warning.
    >


    There is also a VC++ warning for this and you can't prevent it with any
    parenthesis. (C4706 assignment within conditional expression)

    The warning is actually the reason why I got into this argument in the
    first place, because I said we can prevent the warning by declaring the
    var inside the if instead of before.

    cheers,
    Martin
    Martin Ba, Feb 28, 2013
    #11
  12. Martin Ba

    army1987 Guest

    On Thu, 28 Feb 2013 11:23:18 +0100, Martin Ba wrote:

    >> Note that some C/C++ checkers will flag "if (x = GetThing()) ..." with
    >> a warning (note the lack of declaration - this assumes "x" is already
    >> defined and in scope) since it looks very much like a typo with "="
    >> instead of "==". gcc will flag this when you have a lot of warnings
    >> enabled - use "if ((x = GetThing()))" with extra parenthesis to avoid
    >> the warning.
    >>
    >>

    > There is also a VC++ warning for this and you can't prevent it with any
    > parenthesis. (C4706 assignment within conditional expression)


    `if ((x = GetThing()) != 0)`?



    --
    [ T H I S S P A C E I S F O R R E N T ]
    Troppo poca cultura ci rende ignoranti, troppa ci rende folli.
    -- fathermckenzie di it.cultura.linguistica.italiano
    <http://xkcd.com/397/>
    army1987, Feb 28, 2013
    #12
  13. Martin Ba

    Öö Tiib Guest

    On Thursday, 28 February 2013 16:29:40 UTC+2, army1987 wrote:
    > On Thu, 28 Feb 2013 11:23:18 +0100, Martin Ba wrote:
    >
    > `if ((x = GetThing()) != 0)`?


    Your "fix" removed declaration from inside of if that is required to be exactly there by OP. ;)
    Öö Tiib, Feb 28, 2013
    #13
  14. Martin Ba

    James Kanze Guest

    On Wednesday, February 27, 2013 12:41:13 PM UTC, Martin T. wrote:
    > C(++) allows for a simple variable declaration+init in the condition of
    > an if or while:


    > while (T t = x) {
    > }


    > if (int x = f()) {
    > }


    > I find this rather useful for things like:


    > if (const T* pThing = GetThing()) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }


    > However, I have been told by a colleague that this is little
    > used and would surprise and confuse a lot of developers and
    > therefore it's better to use the form:


    > const T* pThing = GetThing();
    > if (pThing) {
    > ...
    > } else {
    > // Handle No-Thing case
    > }


    > Opinions?


    I wouldn't say little used, because a lot of C++ programmers
    seem to like obfuscation (and it was intentionally added to the
    standard, so someone must like it). Still, such compactation
    does nothing to improve readability, and when I see it in code
    I have to work on, the first thing I'll do is to remove it.

    --
    James
    James Kanze, Feb 28, 2013
    #14
  15. Martin Ba

    James Kanze Guest

    On Wednesday, February 27, 2013 1:23:24 PM UTC, Alain Ketterlin wrote:
    > Martin Ba <> writes:


    > [...]
    > > while (T t = x) {
    > > }


    > > if (int x = f()) {
    > > }


    > [instead of]


    > > const T* pThing = GetThing();
    > > if (pThing) {
    > > ...
    > > } else {
    > > // Handle No-Thing case
    > > }


    > They are not equivalent, because the former restricts the scope of the
    > declaration (you would need a pair of braces around the second piece of
    > code to get the same effect).


    If that makes a difference, your functions are too large. And
    note that in the case of `if`...`else`, the variable _is_ in
    scope in the `else` branch.

    > I find myself using this more and more, often changing my code
    > to make these patterns usable. I think you should not worry
    > about potential "surprise and confusion", because the code
    > looks to me completely unambiguous, and therefore it takes
    > almost no time to get used to.


    Anyone trying to understand your code has a right to suppose
    that `if` means `if`, and not `if`, plus define a new variable.
    You can write all of your functions in a single line, too, but
    that doesn't help readability.

    --
    James
    James Kanze, Feb 28, 2013
    #15
  16. Martin Ba

    James Kanze Guest

    On Thursday, February 28, 2013 11:22:26 AM UTC, Andy Champ wrote:
    > On 28/02/2013 10:23, Martin Ba wrote:


    > > On 27.02.2013 15:26, David Brown wrote:
    > >> On 27/02/13 13:41, Martin Ba wrote:
    > >>> C(++) allows for a simple variable declaration+init in the condition of
    > >>> an if or while.....

    >
    > There are two things here - it looks a bit odd to put "if
    > (type var = ...) into code. But the fact that it limits the
    > scope of the variable is a far more powerful argument than it
    > relying on the implicit conversion to bool.


    The implicit conversion to `bool` is, of course, a killer
    argument. This is one of the most confusing features of C/C++;
    and even after more than 30 years of using the language, I still
    get confused by things like `if ( strcmp(...) )`. Even in C, it
    was more less recognized good practice to program as if the
    language had a boolean type, the comparison operators returned
    a boolean type, and there were no implicit conversions to
    `bool`. Those have been the rules in every coding guidelines
    I've seen in those 30 years.

    As for limiting scope: if this is a significant issue, you have
    a larger problem: your functions are too long.

    --
    James
    James Kanze, Feb 28, 2013
    #16
  17. Martin Ba

    Stefan Ram Guest

    James Kanze <> writes:
    >Anyone trying to understand your code has a right to suppose
    >that `if` means `if`


    This sounds strange from someone to whome the grave accent

    <`> 96, Hex 60, Octal 140 GRAVE ACCENT, SPACING GRAVE

    means »quotation mark«.

    »If« means »if«, but not »if«!

    That is, we are using C++, not English.

    »If« in C++, means the C++ »if«, not the English »if«.

    In C++, we read and write code like »if( int const c = ...«,
    in English we do not.
    Stefan Ram, Feb 28, 2013
    #17
  18. Martin Ba

    BCFD36 Guest

    On 2/28/13 9:28 AM, James Kanze wrote:
    > On Wednesday, February 27, 2013 12:41:13 PM UTC, Martin T. wrote:
    >> C(++) allows for a simple variable declaration+init in the condition of
    >> an if or while:

    >
    >> while (T t = x) {
    >> }

    >
    >> if (int x = f()) {
    >> }

    >
    >> I find this rather useful for things like:

    >
    >> if (const T* pThing = GetThing()) {
    >> ...
    >> } else {
    >> // Handle No-Thing case
    >> }

    >
    >> However, I have been told by a colleague that this is little
    >> used and would surprise and confuse a lot of developers and
    >> therefore it's better to use the form:

    >
    >> const T* pThing = GetThing();
    >> if (pThing) {
    >> ...
    >> } else {
    >> // Handle No-Thing case
    >> }

    >
    >> Opinions?

    >
    > I wouldn't say little used, because a lot of C++ programmers
    > seem to like obfuscation (and it was intentionally added to the
    > standard, so someone must like it). Still, such compactation
    > does nothing to improve readability, and when I see it in code
    > I have to work on, the first thing I'll do is to remove it.
    >


    I have read the whole thread. There is some food for thought, but in
    general I like the 2nd method. Reasons:
    1. Limiting the scope is usually not THAT important. If you have
    multiple items with the same thing in the same routine, just in
    different scopes, your code is going to be very confusing.
    2. When debugging, you can breakpoint on the line with the call.
    Depending on the debugger, you may not easily be able to step into
    GetThing(). Stepping out also could get interesting.
    3. As a matter of taste, I don't like doing multiple things on the same
    line, let alone in the same statement. Things can get really muddy.
    4. I make exceptions for for loops. Declaring the iterator in the loop
    statement is pretty clear. Doing one or more function calls within an if
    statement just muddies things up. And the "=" vs "==" can shoot you
    right in the foot. And my feet have enough holes in them, thank you very
    much.

    --
    Dave Scruggs
    Senior Software Engineer, Lockheed Martin
    BCFD36, Mar 1, 2013
    #18
  19. Martin Ba

    Martin Ba Guest

    Implicit conversion to bool - good or bad? (was: Re: Style: Declarationin if/while-condition ...)

    On 28.02.2013 18:37, James Kanze wrote:
    > On Thursday, February 28, 2013 11:22:26 AM UTC, Andy Champ wrote:
    >> On 28/02/2013 10:23, Martin Ba wrote:

    >
    >>> On 27.02.2013 15:26, David Brown wrote:
    >>>> On 27/02/13 13:41, Martin Ba wrote:
    >>>>> C(++) allows for a simple variable declaration+init in the condition of
    >>>>> an if or while.....

    >>
    >> There are two things here - it looks a bit odd to put "if
    >> (type var = ...) into code. But the fact that it limits the
    >> scope of the variable is a far more powerful argument than it
    >> relying on the implicit conversion to bool.

    >
    > The implicit conversion to `bool` is, of course, a killer
    > argument. This is one of the most confusing features of C/C++;
    > and even after more than 30 years of using the language, I still
    > get confused by things like `if ( strcmp(...) )`. Even in C, it
    > was more less recognized good practice to program as if the
    > language had a boolean type, the comparison operators returned
    > a boolean type, and there were no implicit conversions to
    > `bool`. Those have been the rules in every coding guidelines
    > I've seen in those 30 years.
    >


    Care to give any references? (How many have you seen? By whom?)

    T* p;
    ....
    if (p) {
    }

    seems perfectly fine to me.

    > As for limiting scope: if this is a significant issue, you have
    > a larger problem: your functions are too long.
    >


    I see it merely as a minor (vs. significant) issue, but an issue it
    still is, even in short functions.


    cheers,
    Martin
    Martin Ba, Mar 1, 2013
    #19
  20. Martin Ba

    Öö Tiib Guest

    Re: Implicit conversion to bool - good or bad? (was: Re: Style:Declaration in if/while-condition ...)

    On Friday, 1 March 2013 11:48:18 UTC+2, Martin T. wrote:
    > On 28.02.2013 18:37, James Kanze wrote:
    > > Those have been the rules in every coding guidelines
    > > I've seen in those 30 years.

    >
    > Care to give any references? (How many have you seen? By whom?)


    The question you are asking has been answered in C++ FAQ.
    You can find C++ FAQ from http://www.parashift.com/c -faq/

    Please read at least it, NP that you haven't yet seen any
    C++ coding guidelines. Thanks.
    Öö Tiib, Mar 1, 2013
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. -
    Replies:
    12
    Views:
    688
    Remon van Vliet
    Jun 15, 2005
  2. Michael B.

    K&R-Style Function Declarations: Good or Bad?

    Michael B., Dec 8, 2003, in forum: C Programming
    Replies:
    28
    Views:
    1,907
    Dan Pop
    Dec 10, 2003
  3. Hal Vaughan
    Replies:
    1
    Views:
    420
    Mark Space
    Jan 23, 2008
  4. rantingrick
    Replies:
    44
    Views:
    1,205
    Peter Pearson
    Jul 13, 2010
  5. Bill W.
    Replies:
    13
    Views:
    290
    Phillip Gawlowski
    May 9, 2011
Loading...

Share This Page