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

[goran.pusic]

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())

That second one is better solved through refactoring, e.g.:

if (auto p = getValidThing(...))
{
}

and

namespace {
const TYPE* getValidThing(...)
{
auto result = getThing();
if (!result)
return nullptr;
if (!isvalidThing(*result))
return nullptr;
return nullptr;
}
}
Moving complex conditions into trivial helpers has no impact on
performance (will get inlined anyhow), but is often a boon for readability.

Nobody should read a three-line "if". That induces a headache, especially on a Friday! ;-)

G.

 

[Balog Pal]

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.

That's a killer argument indeed. IMNSHO where singlestepping and
breakpointing in a debugger takes up practically measurable amount of
dev time we can hardly speak about quality in the first place.

And tuning code for that activity implies that even hope is abandoned
that it gets better.

 

[James Kanze]

On 28.02.2013 18:37, James Kanze wrote:

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

I'd guess I've seen something like 10. They're all proprietary,
of course, because they were developed in house for the
companies I've worked for. This goes back to C, of course, but
the Ellemtel coding guidelines explicitly forbid it as well.

It's worth noting, too, that in the original proposal to add
`bool` to the language, conversions of pointer to bool were to
be deprecated.

T* p;

if (p) {
}

seems perfectly fine to me.

And I (and most people I've worked with) find it confusing.

 

[Stefan Ram]

And I (and most people I've worked with) find it confusing.

A C++ programmer eventually is expected to understand code like

template< typename A, typename B >
::std::function< B( A )>fix( std::function< std::function
< B( A )>(std::function< B( A )>)> f )
{ F< ::std::function< B( A )>>r =
{ ::std::function< std::function< B( A )>
( F< std::function< B( A )>>) >
([ f ](F< std::function< B( A )>> w )
{ return f( ::std::function< B( A )>([ w ]( A x )
{ return w.o( w )( x ); })); })}; return r.o( r ); }

. If he already finds »if (p)« confusing, well ...

 

[Stefan Ram]

I've been programming in C++ for over a quarter of a century and
I have _zero_ desire to understand code like that.

Admittedly, it also is hard to understand because it is only
a part of a program, not the complete program, so some
definitions are missing. But one can still get the idea of
a distinction between code that is inherently complex and
simple code that just requires a reader to know the language.

 

[jz bnk]

Admittedly, it also is hard to understand because it is only

a part of a program, not the complete program, so some

definitions are missing. But one can still get the idea of

a distinction between code that is inherently complex and

simple code that just requires a reader to know the language.

It seems like fix is being used to terminate a dynamic function chain you're
building (which is kind of neat by the way - why weren't you using something
like the following:

function <B(A)> fix (function <function <B(A)> (function <B(A)>)> f)
{
return [f](A x)
{
return f([](function <B(A)> y) {return y})(x);
};
}

 

[Stefan Ram]

It seems like fix is being used to terminate a dynamic function chain you're
building (which is kind of neat by the way - why weren't you using something
like the following:

That code was stolen from

http://rosettacode.org/wiki/Y_combinator#C.2B.2B

by me, a page, which I found looking for an arbitrary
example of C++ code that is difficult to understand for an
average C++ programmer, which seems to include me.

 

[jz bnk]

That code was stolen from



http://rosettacode.org/wiki/Y_combinator#C.2B.2B



by me, a page, which I found looking for an arbitrary

example of C++ code that is difficult to understand for an

average C++ programmer, which seems to include me.

I misread the above function, so it seems that would include me as well. The
following works, and I think it's a little cleaner than the above:

template <typename A, typename B>
std::function <B(A)> fix (function <function<B(A)>> f)
{
return [=](A x) -> B
{
std::function <B(A)> top;
top = [&](A x) {
return f(top)(x);
};

return top(x);
}
}

although I'm not entirely convinced I'm not doing something awful above like
invoking undefined behaviour - I'll need to give it some more thought. Anyway,
thanks for the link - it was interesting

 

[jz bnk]

std::function <B(A)> fix (function <function<B(A)>> f)

Typo - that should be:
function <B(A)> fix (function <function<B(A)>(function <B(A)>)> f)

 

[Martin Ba]

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.

Right. And now I'm going to search through the whole C++FAQ to find out
what exactly you refer to.

I have skimmed it several times but never read it top-to-bottom. Don't
remember reading an item wrt. the implicit bool conversion - do you have
the detailed link?

cheers,
Martin

 

[Öö Tiib]

Right. And now I'm going to search through the whole C++FAQ to find out
what exactly you refer to.

I meant that there is whole section dedicated to coding standards. You
can apparently read so why not?

I have skimmed it several times but never read it top-to-bottom. Don't
remember reading an item wrt. the implicit bool conversion - do you have
the detailed link?

There can not be such strong rule, implicit conversion to bool is
language feature. In general ... James is right, lot of guidelines,
static analysis tools and even some compilers warn against it. One may
find coding standard for any taste however.

 

[James Kanze]

A C++ programmer eventually is expected to understand code like

template< typename A, typename B >
::std::function< B( A )>fix( std::function< std::function
< B( A )>(std::function< B( A )>)> f )
{ F< ::std::function< B( A )>>r =
{ ::std::function< std::function< B( A )>
( F< std::function< B( A )>>) >
([ f ](F< std::function< B( A )>> w )
{ return f( ::std::function< B( A )>([ w ]( A x )
{ return w.o( w )( x ); })); })}; return r.o( r ); }

. If he already finds »if (p)« confusing, well ...

Just because you can write obfuscated C++, doesn't mean you have
to (although some programmers do seem to think so).

You've heard of the IOCCC, of course. At one point (long before
templates and template metaprogramming, even), some one asked
why there wasn't something similar for C++. I think it was
Steve Clamage who responded that it would be too much like
shooting fish in a barrel.

 

[Seungbeom Kim]

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.

Having short enough functions would be ideal, but in reality we don't
enjoy that all the time. I've had to maintain legacy code which has
functions that are hundreds of lines long, where introducing name into
scope can be a big deal. Some even has "goto quit;", and defining an
object (with ctor/dtor) between the goto and the label will break the
goto, so you have to limit the scope manually:

if (...) goto quit;
// ...
{ // limit the scope of obj
X obj(...);
// ...
}
// ...
quit:

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.

Why is this a problem? No matter whether you have

if (Thing* pThing = getThing()) {
// ...
}

or

Thing* pThing = getThing();
if (pThing) {
// ...
}

you can always set a breakpoint on the line that has the call to
getThing(), and the debugger will stop before the call, and you have
the choice to step into or over the call. (This is based on my
experience with GDB, and other debuggers may vary, but it would be
surprising if any decent one didn't support this.)

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.

I can agree on this. Especially when multiple side effects are involved
and especially more when I care about their order. Less when only a
single side effect is involved (e.g. assigning to an object and
inspecting it); otherwise I'd have to give up neat things like:

while (std::getline(in, str)) { ...}

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.

What about doing one or more function calls in the "condition" part or
the "iteration" part in for loops? (Just curious.)