469,314 Members | 2,207 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 469,314 developers. It's quick & easy.

Declaration of variables

I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!

Mar 6 '06 #1
39 2003
Gaijinco wrote:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!


I think it's your friend who should _prove_ it to you that there is some
advantage to his method.

V
--
Please remove capital As from my address when replying by mail
Mar 6 '06 #2
Gaijinco wrote:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!

Since `aux' is only meaningful inside the loop why not limit its scope
to the block inside the loop?

On the other hand, there are situations where you wouldn't want to do
this, when, for example, the variable in question is of a type that
requires significant construction overhead. This, however, seems not to
be one of those situations. ;-)

HTH,
--ag

--
Artie Gold -- Austin, Texas
http://goldsays.blogspot.com
"You can't KISS* unless you MISS**"
[*-Keep it simple, stupid. **-Make it simple, stupid.]
Mar 6 '06 #3
Gaijinco wrote:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!


Your friend might think that the first code is slower. In some few similar
cases it might indeed be slower. You shouldn't worry about that.

Tell your friend that premature optimization is the root of all evil. The
most important resource to optimize is programmer time. Programmers must
write clean code first, because it's easy to make beautiful code fast than
fast code beautiful.

After your for loop ends, aux's only meaning is "the last variable in the
array". Subsequent statements should not rely on the for loop "leaking" its
last aux value out. If they want the last value in the array, they should
get it themselves. They should decouple as much as possible from the rest
of your program, including the statements just before them.

Always give any identifier the narrowest scope and access possible. Don't
use global variables or public data members. Never create a variable
without giving it an initial value. Put all these rules together, and you
have many reasons to put int aux inside the loop.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 6 '06 #4
Gaijinco posted:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!

Here's how I'd to do it:

for (int aux, i = 0; i < size -1; ++i)
{
...
}
If I had to pick one of the two ways you posted, then I'd put the
variable declaration outside the for loop. Does it make a difference?
Probably not... but at least your safe in the thought that memory isn't
being continuously allocated and deallocated for each iteration of the
loop.

Also as another Artie said, if you dealing with something like an
std::string, then definitely put it *outside* the loop.

The final reason is that I like consistency; if I'm going to put
std::string outside the loop, then I'll put an "int" outside the loop
too.

-Tomás

Mar 6 '06 #5
On 6 Mar 2006 12:13:48 -0800, "Gaijinco" <ga******@gmail.com> wrote:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}
Why? Both versions are essentially the same. The performance is the
same. The second version is bad style because it doesn't obey to the
'minimal scoping rule'.
What do you think? Thaks!


Your friend has gone astray. Lead him back to the right path.
Roland Pibinger
Mar 6 '06 #6
Tomás posted:
Gaijinco posted:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i]; array[i]=array[i+1]; array[i+1]=aux; }

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i]; array[i]=array[i+1]; array[i+1]=aux; }

What do you think? Thaks!

Here's how I'd to do it:

for (int aux, i = 0; i < size -1; ++i)
{
...
}
If I had to pick one of the two ways you posted, then I'd put the
variable declaration outside the for loop. Does it make a difference?
Probably not... but at least your safe in the thought that memory isn't
being continuously allocated and deallocated for each iteration of the
loop.

Also as another Artie said, if you dealing with something like an
std::string, then definitely put it *outside* the loop.

The final reason is that I like consistency; if I'm going to put
std::string outside the loop, then I'll put an "int" outside the loop
too.

-Tomás

Furthermore, if your worried about scope, God gave us curly braces for a
reason:

int main()
{
//Some code

{
int aux;
for (int i; i < size -1; ++i)
{
//Some Code
}
}

//Some more code
}
-Tomás
Mar 6 '06 #7
Tomas wrote:
Here's how I'd to do it:

for (int aux, i = 0; i < size -1; ++i)
{
...
}
Why?

The scope is not as narrow as possible. It's one tick wider, into a scope
where it's not needed.
If I had to pick one of the two ways you posted, then I'd put the
variable declaration outside the for loop. Does it make a difference?
Probably not... but at least your safe in the thought that memory isn't
being continuously allocated and deallocated for each iteration of the
loop.
That's premature optimization.

Further, the earliest definitions of C state that storage for all local
variables in a function allocate when the function enters, regardless of
their scope. I'm sure The Standard has since mutilated that requirement,
but I can't imagine a compiler doing it any other way.
Also as another Artie said, if you dealing with something like an
std::string, then definitely put it *outside* the loop.
Why? Have you time tested it?

What if std::string inside a loop fit in your CPU cache, but outside the
loop it overflowed your cache and caused memory thrashing?
The final reason is that I like consistency; if I'm going to put
std::string outside the loop, then I'll put an "int" outside the loop
too.


Then put it consistently inside the loop. This improves the odds it will go
away!

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 6 '06 #8
"Tom?s" <NU**@null.null> wrote:
The final reason is that I like consistency; if I'm going to put
std::string outside the loop, then I'll put an "int" outside the loop
too.


Found these quotes in the FAQ:

"Consistency is good, but it is not the greatest good" [29.5]

"The real problem is that people tend to worship consistency, and they
tend to extrapolate from the obscure to the common. That's not wise."
[10.19]

(Granted, they are talking about different situations, but nonetheless
they are good pieces of advice to keep in mind in general.)

--
Marcus Kwok
Mar 6 '06 #9
Tomás wrote:
Gaijinco posted:

I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!
Here's how I'd to do it:

for (int aux, i = 0; i < size -1; ++i)
{
...
}
If I had to pick one of the two ways you posted, then I'd put the
variable declaration outside the for loop. Does it make a difference?
Probably not... but at least your safe in the thought that memory isn't
being continuously allocated and deallocated for each iteration of the
loop.

Also as another Artie said, if you dealing with something like an
std::string, then definitely put it *outside* the loop.


I think you've overstated my comments to an extent; I'm not sure I would
call a std::string something that involves `significant construction
overhead'. In any event, as noted elsethread, it's an optimization in
any case and not to be undertaken lightly.
The final reason is that I like consistency; if I'm going to put
std::string outside the loop, then I'll put an "int" outside the loop
too.


It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.

Whenever possible localilty is *good*!

Cheers,
--ag

--
Artie Gold -- Austin, Texas
http://goldsays.blogspot.com
"You can't KISS* unless you MISS**"
[*-Keep it simple, stupid. **-Make it simple, stupid.]
Mar 6 '06 #10
It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.


When I write extra-special long complicated loops, I resort to brute force.
Here's a copy-paste of some of my latest code:
manual_loop_scope:
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration:
{
bool const& condition = i < (NumericInfo::max_divisions_euro -
1);

if ( !condition ) goto manual_loop_end;

manual_loop_body:
{
if ( p_digit[0] || p_digit[1] || p_digit[2] )
{
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, ( p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) );
}

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) );
}
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue:
{
++i;
p_digit += 3;
--i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ;
}
Beauty is irrelevant.

-Tomás
Mar 6 '06 #11

Tomás wrote:
It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.
When I write extra-special long complicated loops, I resort to brute force.
Here's a copy-paste of some of my latest code:
manual_loop_scope:
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration:
{
bool const& condition = i < (NumericInfo::max_divisions_euro -
1);

if ( !condition ) goto manual_loop_end;

manual_loop_body:
{
if ( p_digit[0] || p_digit[1] || p_digit[2] )
{
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, ( p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) );
}

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) );
}
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue:
{
++i;
p_digit += 3;
--i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ;
}


You're joking, right? Have you ever worked on code that's developed or
maintained by more than one person?
Beauty is irrelevant.


If by "beauty" you mean "readability through conformance to accepted
normal practice" then, no it's not irrelevant. It's more important than
anything else.

You're trolling aren't you.

Gavin Deane

Mar 7 '06 #12

"Tomás" <NU**@NULL.NULL> wrote in message
news:Or******************@news.indigo.ie...
It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.


When I write extra-special long complicated loops, I resort to brute
force.
Here's a copy-paste of some of my latest code:
manual_loop_scope:
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration:
{
bool const& condition = i < (NumericInfo::max_divisions_euro -
1);

if ( !condition ) goto manual_loop_end;

manual_loop_body:
{
if ( p_digit[0] || p_digit[1] || p_digit[2] )
{
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, (
p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) );
}

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) );
}
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue:
{
++i;
p_digit += 3;
--i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ;
}
Beauty is irrelevant.

-Tomás


Thank you. I haven't seen a good example of spagetti code for a number of
years.
Mar 7 '06 #13
Gavin Deane posted:

Tomás wrote:
> It's only worth the ugliness of doing that if it's demonstrably
> worth that ugliness in terms of *needed* performance.
When I write extra-special long complicated loops, I resort to brute
force. Here's a copy-paste of some of my latest code:
manual_loop_scope: {
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration: {
bool const& condition = i <
(NumericInfo::max_divisions_euro - 1);

if ( !condition ) goto manual_loop_end;

manual_loop_body: {
if ( p_digit[0] || p_digit[1] || p_digit[2] )
{
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, (
p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) ); }

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) ); }
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue: { ++i;
p_digit += 3; --i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ; }


You're joking, right? Have you ever worked on code that's developed or
maintained by more than one person?


No.

Beauty is irrelevant.


If by "beauty" you mean "readability through conformance to accepted
normal practice" then, no it's not irrelevant. It's more important than
anything else.


I don't go for "accepted normal practice" -- I write my own code my own way.
It's efficient, portable and bug free.

My labels make it quite clear what's going on.
You're trolling aren't you.


No, I'm actually writing loops using labels and the "goto" keyword. If you
find it that scary, maybe you sould Google for "goto" and see what all the
grown-ups do with it.
-Tomás
Mar 7 '06 #14

Artie Gold wrote:
Gaijinco wrote:
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!
Since `aux' is only meaningful inside the loop why not limit its scope
to the block inside the loop?

On the other hand, there are situations where you wouldn't want to do
this, when, for example, the variable in question is of a type that
requires significant construction overhead. This, however, seems not to
be one of those situations. ;-)


Even in those cases you should go for the innermost scope until
performance and experimentation tells you otherwise. If e.g. the
variable was a std::string, semantics differ when you put the string
outside the loop, and the extra work to reset the string at each
iteration might well be comparable to the cost of one constructor and
one destructor.

/Peter
HTH,
--ag

--
Artie Gold -- Austin, Texas
http://goldsays.blogspot.com
"You can't KISS* unless you MISS**"
[*-Keep it simple, stupid. **-Make it simple, stupid.]


Mar 7 '06 #15

Tomás wrote:
Gavin Deane posted:

Tomás wrote:
> It's only worth the ugliness of doing that if it's demonstrably
> worth that ugliness in terms of *needed* performance.

When I write extra-special long complicated loops, I resort to brute
force. Here's a copy-paste of some of my latest code:
[snip what supposedly should be C++ code}

You're joking, right? Have you ever worked on code that's developed or
maintained by more than one person?
No.

Beauty is irrelevant.


If by "beauty" you mean "readability through conformance to accepted
normal practice" then, no it's not irrelevant. It's more important than
anything else.


I don't go for "accepted normal practice" -- I write my own code my own way.
It's efficient, portable and bug free.

My labels make it quite clear what's going on.
You're trolling aren't you.


No, I'm actually writing loops using labels and the "goto" keyword. If you
find it that scary, maybe you sould Google for "goto" and see what all the
grown-ups do with it.


These must be the same grown-ups that never take back-ups and don't do
unittests?
Well.... theres not much worth backing up anyway.

/Peter


-Tomás


Mar 7 '06 #16
Tomás wrote:
No, I'm actually writing loops using labels and the "goto" keyword. If you
find it that scary, maybe you sould Google for "goto" and see what all the
grown-ups do with it.


Instead of priding yourself on obfuscation, researching how to write the
_simplest_ possible code for a set of features will improve the odds that
you could add value to a team of software engineers.

Specifically, code that is absurdly elaborate in one direction becomes
hopelessly resistant to unexpected changes in other directions. Preserving
flexibility is the heart of the software process because it allows customers
to request unexpected new features.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #17

Tomás wrote:
Gavin Deane posted:
Tomás wrote:
Beauty is irrelevant.


If by "beauty" you mean "readability through conformance to accepted
normal practice" then, no it's not irrelevant. It's more important than
anything else.


I don't go for "accepted normal practice" -- I write my own code my own way.
It's efficient, portable and bug free.


Which proves my point that you've never had to work on a code base with
more than one developer. You left out the most important factor -
readability. Yes, that is more important than bug-free.

If you write clear code that conforms to the principle of least
surprise, but it has bugs, I can understand the code and fix the bugs,
and modify the code with a low risk of introducing new bugs, all very
easily. If you write the sort of obfuscated code you posted in this
thread and it has no bugs, the chance of me being able to modify it
without introducing new bugs, and the chance of quickly identifying and
fixing those bugs are greatly reduced.
You're trolling aren't you.


No, I'm actually writing loops using labels and the "goto" keyword. If you
find it that scary, maybe you sould Google for "goto" and see what all the
grown-ups do with it.


If that's not trolling I don't know what is. Please stop.

Gavin Deane

Mar 7 '06 #18
TB
Tomás skrev:
It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.


When I write extra-special long complicated loops, I resort to brute force.
Here's a copy-paste of some of my latest code:
manual_loop_scope:
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration:
{
bool const& condition = i < (NumericInfo::max_divisions_euro -
1);

if ( !condition ) goto manual_loop_end;

manual_loop_body:
{
if ( p_digit[0] || p_digit[1] || p_digit[2] )
{
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, ( p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) );
}

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) );
}
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue:
{
++i;
p_digit += 3;
--i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ;
}


I'm speechless... ugh

--
TB @ SWEDEN
Mar 7 '06 #19
TB wrote:
goto manual_loop_begin_iteration;
}//close manual_loop_continue
manual_loop_end: ;
}


I'm speechless... ugh


I have seen worse. Take a programmer who grew up with assembly, and let them
write C. Then, without a little education, and besotted with the belief that
"goto is faster", they might write an entire, brilliant application using
assembly-style C, with all kinds of run-on functions, looping with goto,
etc.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #20
TB
Phlip skrev:
TB wrote:
goto manual_loop_begin_iteration;
}//close manual_loop_continue
manual_loop_end: ;
}

I'm speechless... ugh


I have seen worse. Take a programmer who grew up with assembly, and let them
write C. Then, without a little education, and besotted with the belief that
"goto is faster", they might write an entire, brilliant application using
assembly-style C, with all kinds of run-on functions, looping with goto,
etc.


Yes, that's true, but this is written, *I assume*, by a C++ programmer.

--
TB @ SWEDEN
Mar 7 '06 #21
Gavin Deane posted:
If you write clear code that conforms to the principle of least
surprise, but it has bugs, I can understand the code and fix the bugs,
and modify the code with a low risk of introducing new bugs, all very
easily. If you write the sort of obfuscated code you posted in this
thread and it has no bugs, the chance of me being able to modify it
without introducing new bugs, and the chance of quickly identifying and
fixing those bugs are greatly reduced.


I think my code is quite clear. Its labels show what part of the code
corresponds to what part of a "domestic loop". Take a domestic "for" loop:

for (unsigned i = 0; i < some_value; ++i)
{
if ( 3 == i ) continue;

DoSomeStuff();
}
Using "goto" and labels, this can be broken into segments as follows:

loop:
{
unsigned i = 0;

loop_begin_iteration:
{
bool const &condition = (3 == i);

if ( !condition ) goto loop_end;

loop_body:
{
if ( 3 == i ) goto loop_continue;

DoSomeStuff();
}
}

loop_continue:
{
++i;
goto loop_begin_iteration;
}

loop1_end: ;
}
The method I show above is very useful if the "continue" part of the loop
(i.e. the part executed after each iteration) is more complex than simply
"++i". It could be:

FiveToThePowerOf( ++i ) += some_value - 17;
some_global_variable = some_value / i;
SomeFunction( some_value / i );

Try fit that nicely into a "for" loop:

for (int i = 0; i < some_value; FiveToThePo...

Also, you could have several more "loop variables" than just i, and these
objects could have long constructor lists.

It seems to me that people who object to my code are the ones who stick to
their non-adventurous "it's not efficent but it gets the job done"
techniques. You can spot them a mile away by their usage of "i++".
> You're trolling aren't you.


No, I'm actually writing loops using labels and the "goto" keyword. If
you find it that scary, maybe you sould Google for "goto" and see what
all the grown-ups do with it.


If that's not trolling I don't know what is. Please stop.

Actually I find it quite arrogant and rude to accuse people of trolling when
such accusation is wholly unwaranted -- it makes you come across as a right
prick. I'm here to talk about C++; what about you?

-Tomás
Mar 7 '06 #22
TB posted:
Phlip skrev:
TB wrote:
goto manual_loop_begin_iteration;
}//close manual_loop_continue
manual_loop_end: ; }

I'm speechless... ugh


I have seen worse. Take a programmer who grew up with assembly, and
let them write C. Then, without a little education, and besotted with
the belief that "goto is faster", they might write an entire,
brilliant application using assembly-style C, with all kinds of run-on
functions, looping with goto, etc.


Yes, that's true, but this is written, *I assume*, by a C++ programmer.

Fully portable, well-defined code with appropriately named objects and
labels.

What exactly is the complaint?

-Tomás
Mar 7 '06 #23
Tomas wrote:
> goto manual_loop_begin_iteration;
> }//close manual_loop_continue
> manual_loop_end: ; }
What exactly is the complaint?


I'm not as smart as you, so I don't understand it.

That's what.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #24

Tomás wrote:
Actually I find it quite arrogant and rude to accuse people of trolling when
such accusation is wholly unwaranted -- it makes you come across as a right
prick.
I couldn't agree more. However, since such an accusation is entirely
warranted in this case I fail to see the relevance of your statement.
I'm here to talk about C++; what about you?


You're missing a fundamental point here. I'm talking about C++ code
intended to be read by people other than yourself - other developers on
the same project, or people reading a newsgroup you post to for
example. If you're writing your own code, for your own projects, by
yourself, and you choose to adopt styles and techniques that for good
reason have little or no correlation with those employed in the world
of collaborative and professional software development, that's none of
my or anyone else's business.

But to continue to assert with more and more vehemence and wilder and
wilder examples, that your techniques are clear, useful and grown-up is
either ignorance or trolling.

Gavin Deane

Mar 7 '06 #25
But to continue to assert with more and more vehemence and wilder and
wilder examples, that your techniques are clear, useful and grown-up is
either ignorance or trolling.


When I finish my current program, I'll show everyone:

A) The size of the executable
B) How fast the program runs
C) How much memory it consumes

Then I'll welcome you to deny the efficacy of my techniques.

-Tomás
Mar 7 '06 #26
Phlip posted:
Tomas wrote:
>> goto manual_loop_begin_iteration;
>> }//close manual_loop_continue manual_loop_end: ; }

What exactly is the complaint?


I'm not as smart as you, so I don't understand it.

That's what.


Why does the conversation degrade to this level?

I'm asking a genuine question here. If I gave the labels names such as
"label1", "label2" and "label3", then I'd acknowledge that the code isn't
particularly concise. But I've given concise labels which have strong
correlation to how a domestic loop is actually implemented.

Furthermore, if I had several nested loops, and the execution of code had to
go back to different points in different loops, then I'd find "goto"
statements preferable.

Spaghetti code isn't taboo. If you have a very complex problem, you'll have
very complex code. If you have a spaghetti problem, you'll have spaghetti
code.

However if I'd working with simple loops, I'll go with "for", "do" and
"while".

-Tomás
-Tomás
Mar 7 '06 #27
On 2006-03-07, Tomás <NU**@NULL.NULL> wrote:
Gavin Deane posted:
If you write clear code that conforms to the principle of least
surprise, but it has bugs, I can understand the code and fix the bugs,
and modify the code with a low risk of introducing new bugs, all very
easily. If you write the sort of obfuscated code you posted in this
thread and it has no bugs, the chance of me being able to modify it
without introducing new bugs, and the chance of quickly identifying and
fixing those bugs are greatly reduced.
I think my code is quite clear. Its labels show what part of the code
corresponds to what part of a "domestic loop". Take a domestic "for" loop:

for (unsigned i = 0; i < some_value; ++i)
{
if ( 3 == i ) continue;

DoSomeStuff();
}
Using "goto" and labels, this can be broken into segments as follows:


Why do you prefer the much more verbose version below?

I agree your loop is more general, but the distance between first
glance and understanding is much larger than in the above. I have to
read and intern five labels and an extra variable to begin to
understand it; that's before I spend time deciphering the execution
order.

Moreover, your loop structure below led you to make an error in your
translation, confusing the continue condition and the loop condition.
loop:
{
unsigned i = 0;

loop_begin_iteration:
{
bool const &condition = (3 == i);

if ( !condition ) goto loop_end;

loop_body:
{
if ( 3 == i ) goto loop_continue;

DoSomeStuff();
}
}

loop_continue:
{
++i;
goto loop_begin_iteration;
}

loop1_end: ;
}

The method I show above is very useful if the "continue" part of the loop
(i.e. the part executed after each iteration) is more complex than simply
"++i". It could be:

FiveToThePowerOf( ++i ) += some_value - 17;
some_global_variable = some_value / i;
SomeFunction( some_value / i );

Try fit that nicely into a "for" loop:

for (int i = 0; i < some_value; FiveToThePo...
Not a problem.

for (int i = 0; i < some_value; FiveToThePowerOf(++i) += some_value - 17)
{

But that's a nonsensical example, anyway, and would generally be
written like this, if at all.

for (int i = 0; i < some_value; ++i)
{
FiveToThePowerOf(i) += some_value-17;
}

I must assume that FiveToThePowerOf has important side-effects.
Also, you could have several more "loop variables" than just i, and
these objects could have long constructor lists.
Not a big deal. for loops may be split up into multiple lines if
necessary.
It seems to me that people who object to my code are the ones who
stick to their non-adventurous "it's not efficent but it gets the
job done" techniques. You can spot them a mile away by their usage
of "i++".


Cleverness and adventurousness *are* very much valued in programming,
but only where applicable. 98% of the time, they are not. It is
usually better to write idiomatic, understandable, refactorable code,
using well-known, trusted, published algorithms and patterns. In
particular, leave the development of new looping constructs to the
langauge designers. If you need something more general than "for", use
"while".

That's not to say that programmers with highly individual style have
never been susccessful. See if you can find the original Bourne shell
code for some interesting reading. See if you still think
adventurousness and cleverness are worth all that much afterwards. See
if you can see why it had to be totally reimplemented before it could
be extended by anyone other than Bourne.

--
Neil Cerutti
Weight Watchers will meet at 7 p.m. Please use large double door
at the side entrance. --Church Bulletin Blooper
Mar 7 '06 #28
Tomas wrote:
When I finish my current program, I'll show everyone:

A) The size of the executable
B) How fast the program runs
C) How much memory it consumes


Can we ask you to add a new feature, and then time _you_, not the program?

Anyone here can perform the same experiment; writing code in a bitterly
complex style, while time-testing each statement to see how it works with a
specific compiler and CPU.

If you want to _really_ learn something, you could disassemble each
function, study the opcodes, and look up how many CPU cycles each one
takes. Then you could optimize those directly, and paste the result back
into your C++ code inside an __asm or similar directive.

Real performance tuning should only obfuscate that 20% of the code where the
CPU spends 80% of its time. Not all of it. And it shouldn't obfuscate so
badly that new features get hard to write.

And the program's footprint is kind'a irrelevant in an era where you can
store >100 MP3s in a cufflink.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #29
Tomás wrote:
Spaghetti code isn't taboo. If you have a very complex problem, you'll
have very complex code. If you have a spaghetti problem, you'll have
spaghetti code.


What problem could possibly compell you to write the loop statement you did?

Post its unit tests and we will see if we can pass them with simpler code.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #30
Tomás wrote:
I'm not as smart as you, so I don't understand it.
Why does the conversation degrade to this level?


That's not a joke or a flame - it's the absolute core of the problem. The
dumber my code is, the more features a dumb person like me (and my team)
can rapidly add to it.

We have a choice between writing clever code and writing code cleverly. If I
choose the latter, and write clever-clever code that's hard to read, I have
placed a ceiling of complexity beyond which I can't add new features. For
someone smarter than me, that ceiling is indeed higher. But we could both
write code with no ceiling, no matter how dumb we are.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #31

Tomás wrote:
But to continue to assert with more and more vehemence and wilder and
wilder examples, that your techniques are clear, useful and grown-up is
either ignorance or trolling.
When I finish my current program, I'll show everyone:
A) The size of the executable


Compared to human readability of the code, the size of the executable
is of little concern[*].
B) How fast the program runs
Compared to human readability of the code, the speed of the program is
of little concern[*].
C) How much memory it consumes
Compared to human readability of the code, memory consumption is of
little concern[*].
Then I'll welcome you to deny the efficacy of my techniques.


Again, I'm talking about collaborative and professional software
development. As I made that clear in my previous post, I assume you are
happy with the context.

Since the criticism of your techniques has nothing to do with their
effect on any of your three benchmarks, showing them to everyone will
not help your argument in the slightest. Against the one benchmark that
matters, your techniques have already been shown to be lacking. The
simple fact that several posters have objected to your style while none
have defended it is the only evidence required.
[*] Unless and until you have proof that your code works but fails to
meet a requirement in one of these area. Then and only then is it
necessary to restructure the code. Readbility must be retained to as
great an extent as is possible.

Gavin Deane

Mar 7 '06 #32
Tomás wrote:
It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.
When I write extra-special long complicated loops, I resort to brute force.
Here's a copy-paste of some of my latest code:
manual_loop_scope:
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration:
{
bool const& condition = i < (NumericInfo::max_divisions_euro - 1);
if ( !condition ) goto manual_loop_end;


I have a very strange feeling about the two lines above.

manual_loop_body:
{
if ( p_digit[0] || p_digit[1] || p_digit[2] )
{
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, ( p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) );
}

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) );
}
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue:
{
++i;
p_digit += 3;
--i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ;
}


The "code" you've posted is hardly readable. Why do you use it instead
of the following (which is not only equivalent, but also correct)?

Are you trying to optimize? The compiler can often do that better than
you...

<code>
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;
for (;
i < (NumericInfo::max_divisions_euro - 1);
++i, p_digit += 3, --i_word)
{
if ( p_digit[0] || p_digit[1] || p_digit[2] ) {
if (others_beforehand)
StringSystem::AppendPALIncCounter(/*...*/);

others_beforehand = true;
StringSystem::AppendPALIncCounter(/*...*/);
}
}
}
</code>
Mar 7 '06 #33
Martin Vejnar wrote:
bool const& condition = i < (NumericInfo::max_divisions_euro
- 1); if ( !condition ) goto manual_loop_end;
I have a very strange feeling about the two lines above.


Why is 'condition' a constant boolean _reference_? To refer to what?
Are you trying to optimize? The compiler can often do that better than
you...


In this case, the compiler can't always guess which values it can alias into
a register, and which might change unseen. Tomas may have profiled, or
might be guessing.

Tomas, if you feel picked on, keep in mind we are all just practicing
spotting and thwarting code like this.

When programming is a profession, not a hobby, elegance matters. Everyone
here (except you) has seen potentially lucrative projects driven into the
ground. Not by bad management, or poor market performance, but by some
horrible codebase of crappy code install by some early guru. Being paid to
work with such code, but not fix it with a clue-bat, is a torment and soon
a nightmare.

http://c2.com/cgi/wiki?JobSecurity

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 7 '06 #34
Tomás wrote:
But to continue to assert with more and more vehemence and wilder and
wilder examples, that your techniques are clear, useful and grown-up is
either ignorance or trolling.


When I finish my current program, I'll show everyone:

A) The size of the executable
B) How fast the program runs
C) How much memory it consumes


Your code *may* -- and note the word *may* -- be small and fast (a good
optimizing compiler might be better), but you fail in what I call the
"X-abilities".

* Readability
* Maintainability
* Scalability

In the "real world", those three are much more important than any minor
"performance improvements" you might get out of that. If I inherited a
piece of code like that, the first thing I'd do is ask my manager if the
author was still working here, and if so, why.

The second thing I'd do is rewrite it (and it wouldn't be hard to get
management blessing for it) so the next person after me could understand
what's going on.

Mar 7 '06 #35
In article <1141676028.834827.318270
@p10g2000cwp.googlegroups.com>, ga******@gmail.com
says...
I have always felt that you should only declared variables as needed,
and implicitily it seems many authors to encourage it, but the other
day a friend told me that declaring variables inside a loop wasn't good
practice, something like:

for(int i=0; i<size-1; ++i){
int aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

should be written like:

int aux;
for(int i=0; i<size-1; ++i){
aux = array[i];
array[i]=array[i+1];
array[i+1]=aux;
}

What do you think? Thaks!


For this exact example, neither. I'd advise:

std::rotate(array, array+1, array+size);

I realize this was meant only as an example though. That
being the case, I'd say your method is preferable as a
rule. In the more general case, if aux was a type that
was expensive to construct and/or destroy, but
(relatively) cheap to assign, and a profiler told you
constructing and destroying all those objects was causing
a significant slow-down in your code, then it might make
sense to hoist it out of the loop.

--
Later,
Jerry.

The universe is a figment of its own imagination.
Mar 7 '06 #36
In article <QL******************@news.indigo.ie>,
NU**@NULL.NULL says...

[ ... ]
I think my code is quite clear.
Unfortunately, you're simply wrong.
Its labels show what part of the code corresponds to
what part of a "domestic loop". Take a domestic "for"
loop:
When I see a for loop, I read it directly. When I see
your code, I first have to translate from the mess you've
made to a for loop, then read and understand that.

Maximizing readability means minimizing the number of
translations between seeing and understanding. By adding
a translation, you've reduced readability.
Using "goto" and labels, this can be broken into segments as follows:
Every construct in every Turing complete language has an
equivalent in any other Turing complete language. We all
know that REAL programmers can write FORTRAN in any
language, but you're the first I've heard of who was
intent on writing Intercal in C++.

[ ... ]
It seems to me that people who object to my code are the ones who stick to
their non-adventurous "it's not efficent but it gets the job done"
techniques. You can spot them a mile away by their usage of "i++".
Rule #1 is that efficiency almost never matters -- but
when it does matter, it almost always matters a lot.

Rule #2 is that micro-optimization rarely improves
efficiency much. Major gains in efficiency usually depend
on algorithmic changes that (in turn) depend on
understanding of the task. Your code style (and in use
the term loosely) obfuscates the task at hand to the
point that it makes those major gains substantially more
difficult to achieve.

To get a noticeable improvement in efficiency from a
goto, you must use it to create a flow of control that
can't be created directly otherwise. Compilers are quite
efficient at actually translating source code to
executable, so to accomplish something, you need to tell
it something it couldn't deduce on its own -- and it most
certainly can deduce exactly where to put labels for the
beginning and end of a loop and such.

My advice would be to spend some reading the assembly
language produced by your compiler for various
constructs. This will help you understand when you really
can gain something by micro-optimizations on the level of
using a goto.

[ ... ]
Actually I find it quite arrogant and rude to accuse people of trolling when
such accusation is wholly unwaranted -- it makes you come across as a right
prick. I'm here to talk about C++; what about you?


If you want to talk about C++, please feel free to start
anytime. So far, you seem to be talking about Intercal
written in C++.

--
Later,
Jerry.

The universe is a figment of its own imagination.
Mar 7 '06 #37
On Mon, 06 Mar 2006 23:19:10 +0000, Tomás wrote:
It's only worth the ugliness of doing that if it's demonstrably worth
that ugliness in terms of *needed* performance.


When I write extra-special long complicated loops, I resort to brute
force. Here's a copy-paste of some of my latest code:

manual_loop_scope:
{
std::size_t i = 0;
std::size_t i_word = NumericInfo::max_divisions_euro - 2;

manual_loop_begin_iteration:
{
bool const& condition = i < (NumericInfo::max_divisions_euro -
1);

if ( !condition ) goto manual_loop_end;

manual_loop_body:
{
if ( p_digit[0] || p_digit[1] || p_digit[2] ) {
if (others_beforehand)
{
StringSystem::AppendPALIncCounter( p_char, (
p_digit
[0] ? lang_attribs.separator_comma : lang_attribs.separator_and ) );
}

others_beforehand = true;
StringSystem::AppendPALIncCounter( p_char,
GetSubThousandWithUnitName( p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word] ) );
}
} // close manual_loop_body
}//close manual_loop_iteration

manual_loop_continue:
{
++i;
p_digit += 3;
--i_word;

goto manual_loop_begin_iteration;

}//close manual_loop_continue

manual_loop_end: ;
}
Beauty is irrelevant.


But surely clarity is? This is one of the oddest bits of code I've seen
in a long time. I felt sure you must be playing some really clever tricks
to make it worth being so obscure, but the more I looked (and it took
quite a bit of looking!) the more it looked like the most simple while (or
for) loop.

I think you have written:

std::size_t i_word = NumericInfo::max_divisions_euro - 2;
while (i_word >= 0) {
if (p_digit[0] || p_digit[1] || p_digit[2]) {
if (others_beforehand) {
StringSystem::AppendPALIncCounter
(p_char,
(p_digit[0] ?
lang_attribs.separator_comma :
lang_attribs.separator_and));
}
others_beforehand = true;
StringSystem::AppendPALIncCounter
(p_char,
GetSubThousandWithUnitName
(p_digit[0], p_digit[1], p_digit[2],
lang_attribs.unit_names[i_word]));
}
i_word -= 1;
p_digit += 3;
}

which is much clearer to me. I can see the loop bounds, when you exit and
what is done with each set of three digits. I am worried that the
initialisation of others_beforehand is not visible, so this loop's
behaviour is tied to earlier code (maybe deliberately) but it means I
can't try to tidy up the inner test.

I can't see any justification for writing your version.

--
Ben.
Mar 8 '06 #38
Jerry Coffin wrote:
std::rotate(array, array+1, array+size);


If you were on my team, you may write that at any time. However...

I currently work with a team using A> legacy code, B> STLport, and C> not as
much cumulative STL chops as you. So I will not put that in, even in the
rare case that we need to rotate an array. I will write stupid code that
obviously works well, and move on. Over time, if this code indeed has a
future, it would naturally migrate in that direction.

But honest thanks for showing off your STL!

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
Mar 8 '06 #39
In article <nZqPf.19224$NS6.4499
@newssvr30.news.prodigy.com>, ph******@yahoo.com says...
Jerry Coffin wrote:
std::rotate(array, array+1, array+size);
If you were on my team, you may write that at any time. However...

I currently work with a team using A> legacy code, B> STLport,
and C> not as much cumulative STL chops as you. So I will not
put that in, even in the rare case that we need to rotate an
array.


That's too bad, if entirely understandable.
I will write stupid code that
obviously works well, and move on. Over time, if this code indeed has a
future, it would naturally migrate in that direction.
We can hope anyway -- the standard algorithms are (IMO)
quite under-utilized. Far too many people don't even
attempt to look beyond std::for_each, which certainly has
its uses, but equally certainly isn't the sole algorithm
worth using.
But honest thanks for showing off your STL!


Oh, come now -- if I was going to show off, I'm sure I
could find something more than the simplest possible
application of an algorithm to do it with. :-)

Seriously though, most of what's in <algorithm> isn't
exactly earth-shaking, but a lot of it can be handy
anyway.

--
Later,
Jerry.

The universe is a figment of its own imagination.
Mar 8 '06 #40

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

83 posts views Thread by Alexander Zatvornitskiy | last post: by
2 posts views Thread by Thomas Matthews | last post: by
2 posts views Thread by Nils Emil P. Larsen | last post: by
7 posts views Thread by Peter Merwood | last post: by
18 posts views Thread by noridotjabi | last post: by
25 posts views Thread by venky | last post: by
18 posts views Thread by sunny | last post: by
14 posts views Thread by subramanian100in | last post: by
1 post views Thread by CARIGAR | last post: by
reply views Thread by zhoujie | last post: by
reply views Thread by harlem98 | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.