Exiting from loop

[vineoff]

Thank you all guys!

 

[Neil Cerutti]

* Kai-Uwe Bux -> Neil Cerutti:

One crucial thing here is the name change, to reflect what the
function produces. I don't think I could guess from the name
'ranged_prompt' (the OP's choice) that it actually does input.
I'd guess it produced a prompt.

Agreed on the function name. I changed it from "f" at the last
minute, and didn't think about it very much.

Possibly "get_ranged_input"? A better version would receive an
istream& instead of hardcoding std::cin, as well.

Of course for good measure -- less Undefined Behavior, those
iostreams aren't my favorite beasts!

Ah, yes. I forgot to check for eof. No sense clearing the stream
if std::cin.eof(). This reveals that the function signature is
probably broken, since I've only left myself the option of
throwing an exception in that case.

-- one should read in a
whole line using getline, and parse it using strtol or strtod
as appropriate (not a stringstream)...

Unfortunately, strtol or strtod are unsuitable except in some
specialization, since we don't know the type of T. The function
should conceivably work with any type supporting <, > and streams.

Here's a new version that's hopefully improved.

/* Leaves junk in result if is.eof() */
template <typename T>
std::istream& get_ranged_input_with_prompt(std::istream& os
, std::istream& is, T& result, const T& min, const T& max)
{
prompt:
os << "Enter a value from " << min << " to " << " max: ";
os.flush();
is >> result;
if (!is && !is.eof()) {
if (result < min || result > max) {
is.clear();
is.ignore(std::numeric_limits<int>::max(), '\n');
goto prompt;
}
}
return is;
}

Hmmm.

 

[Jack Saalweachter]

It is not efficient.

In this situation, you seem to be iterating once over three coordinates.
Has anyone here ever tried defining an object to perform the
iteration? There are several ways to do it, but one might involve
ultimately writing it like so:

for (multi_iterator<int, int, int> i(10, 10, 10); i; ++i) {
// each of the three values of i are used.
break; // Exits the loop.
}

In such a case, the iterator would know its bounds. Iteration would
handle rollover, and evaluating it as a bool would tell whether it was
still within its bounds. If you wrap a tuple, you get all sorts of
simple accessors and extendability for free.

 

[int2str]

A Do-It-Yourself RAII-based version might go like this:

Very nice solution

StreamCleanup cleaner( std::cin );

Warning (line N): Unused variable 'cleaner' ...



How do you work around that warning (without introducing some silly
cleaner.noop(); )?
I encounter exactly that problem in my code every once in a while.

Cheers,
Andre

 

[Alf P. Steinbach]

* Andre ([email protected]):

* Alf P. Steinbach:

Very nice solution


Warning (line N): Unused variable 'cleaner' ...



How do you work around that warning (without introducing some silly
cleaner.noop(); )?

Warnings are compiler-specific, and most workarounds for them also.

I guess a macro would be thing here, e.g., for g++, off the cuff (might not
work)

#ifdef __GNUG__
#define UNUSED( x ) x __attribute__((unused))
#else
#define UNUSED( x ) x
#endif

#define UNIQUE_NAME //something, using __LINE__ (use __COUNTER__ for MSVC)

#define GUARD( t, args ) t UNUSED( UNIQUE_NAME ) = t args;

...
GUARD( StreamCleanup, (std::cin) );

Or, you can use the compiler option to suppress this particular warning
(again, that depends on the compiler).

Btw., the macro above relies on "()" being valid as a macro argument for the
case of a constructor with no arguments. I don't know whether it really is.
But it's always worked, and the proof is in the pudding, as they say.

 

[mlimber]

It is not efficient.

Did you profile it? Programmers are terrible at judging what is
inefficient and what the compiler can optimize away. Readability and
maintainability, not efficiency, should be the primary concern until a
profiler determines that a certain part of the code should be reworked
for efficiency. Premature optimization is the root of all kinds of
evil, after all. As Herb Sutter said:

"By and large, programmers--that includes you and me--are notoriously
bad at guessing the actual space/time performance bottlenecks in their
own code. If you don't have performance profiles or other empirical
evidence to guide you, you can easily spend days optimizing something
that doesn't need optimizing and that won't measurably affect runtime
space or time performance. What's even worse, however, is that when you
don't understand *what* needs optimizing you may actually end up
pessimizing (degrading your program) by of saving a small cost while
unintentionally incurring a large cost. Once you've run performance
profiles and other tests, and you actually know that a particular
optimization will help you in your particular situation, then it's the
right time to optimize." (http://www.gotw.ca/publications/mill09.htm)

Cheers! --M

 

[=?iso-8859-1?Q?Ali_=C7ehreli?=]

If I'm having nested loops like:

for (...) for (..) for (...) { /* exit here */ }

and I need to exit from there ^ .

Is it better to use exceptions or goto or some other method?

How about an extra boolean added to each condition:

bool done = false;

for (int i = 0; !done && (i != 3); ++i)
{
for (int j = 0; !done && (j != 3); ++j)
{
// ... etc.

done = /* some exit condition */;
}
}

Ali

 

[Andrew Koenig]

There are some situations in which a "goto" is the most readable
and clear way to go. A good example would be the initialization
of a device driver (ok, those are normally done in C and not
C++). If something goes wrong there you must make sure that you
revert the state of the device on how you found it.

enum subdevice{A, B, C};

bool init_device()
{
sub_device_status pre_A;
sub_device_status pre_B;
sub_device_status pre_C;

// each init_subdevice_ call initializies the
// internal data structures and then reset the
// subdevice

get_device_status(A, &pre_A);
if( !init_subdevice_A() )
goto failed_A;

get_device_status(B, &pre_B);
if( !init_subdevice_B() )
goto failed_B;

get_device_status(C, &pre_C);
if( !init_subdevice_C() )
goto failed_C;

// All subdevices got initialized properly
return true;

failed_C:
set_device_status(C, pre_C);
failed_B:
set_device_status(B, pre_B);
failed_A:
set_device_status(A, pre_A);

return false;
}

Frankly I don't know a cleaner way to write this, but I'm open to
other suggestions.

get_device_status(A, &pre_A);
if( init_subdevice_A() ) {

get_device_status(B, &pre_B);
if( init_subdevice_B() ) {

get_device_status(C, &pre_C);
if( init_subdevice_C() ) {

// All subdevices got initialized properly
return true;

}
set_device_status(C, pre_C);
}
set_device_status(B, pre_B);
}
set_device_status(A, pre_A);

return false;

 

[=?iso-8859-1?Q?Ali_=C7ehreli?=]

1. For bailing out of deeply nested loops, GOTO is not necessarily
harmful. But I don't have my copy of the Standard available, and am not
sure if a goto calls destructors when exiting blocks.

Destruction is only one side of the coin. How about construction:

for (;
{
goto future;
}

Foo foo;

future:

cout << "doomed\n";

Unfortunately foo will not be constructed in the future.

To the OP: goto is harmful. There is plenty of writing about that, which can
be easily found on the Net.

Ali

 

[Guest]

Prompting and verifying input is a case where goto turns out to
seem nice and clean.

Using goto is not nice nor clean. goto is inherently risky and the code
below is obfuscated.

[...]

template <typename T>
T ranged_prompt(T min, T max)
{
T n;
prompt:
std::cout << "Enter a number from " << min << " to " << max ": ";
std::cout.flush()

Aside: flush is unnecessary above. Inputting from cin will cause that anyway
because cin is "tied" to cout (see std::tie).

std::cin >> n;
if (!std::cin || n < min || n > max) {
std::cin.clear();
std::cin.ignore(std::numeric_limits<int>::max(), '\n');
goto prompt;
}
return n;
}

What you do above is "while the input is not good, [...]." What other method
can be more clear than

bool good_input = false;

while ( ! good_input)
{
/* ... */
}

Ali

 

[red floyd]

Destruction is only one side of the coin. How about construction:

for (;
{
goto future;
}

Foo foo;

future:

cout << "doomed\n";

Unfortunately foo will not be constructed in the future.

To the OP: goto is harmful. There is plenty of writing about that, which
can be easily found on the Net.

Sure, but for the purpose of *BAILING OUT OF A DEEPLY NESTED LOOP*

for (...)
{
for (...)
{
for (...)
{
...
if (disaster)
goto bailout;
}
}
}
bailout:

Note that what you propose can occur *WITHOUT* goto, as well:

switch (expr)
{
case 1:
Foo foo;
case 2:
// uh oh, if expr == 2, foo is not constructed!
foo.bar();
break;
default:
break;
}

 

[=?iso-8859-1?Q?Ali_=C7ehreli?=]

use break

What are you talking about? Use break when? Where? In what context?

Please quote enough of the previous post to make what you say clear.
[Off-topic rant: Google may be hosting the newsgroups today; but that
doesn't mean that they understand the concept.]

If you suggested to "use break" to exit out of multiple loops; that is plain
wrong.

On the other hand, if you suggested to "use break" to take square root of a
double, that is ridiculous...

Ali

 

[TIT]

Ali Çehreli sade:
It does call destructors.

Destruction is only one side of the coin. How about construction:

for (;
{
goto future;
}

Foo foo;

If Foo is a POD then this is legal, it will exist and
work properly. (6.7/3)

future:

cout << "doomed\n";

Unfortunately foo will not be constructed in the future.

In any other case this jump is according to the standard
ill-formed, as such not well-formed and therefore undefined
behavior.

To the OP: goto is harmful. There is plenty of writing about that, which
can be easily found on the Net.

Gotos are sometimes quite practical.

TIT

 

[Alf P. Steinbach]

* =?iso-8859-1?Q?Ali_=C7ehreli?=:

Destruction is only one side of the coin. How about construction:

for (;
{
goto future;
}

Foo foo;

future:

cout << "doomed\n";

Unfortunately foo will not be constructed in the future.

Happily C++ protects against that: that particular goto is not allowed,
para 6.7/3.

To the OP: goto is harmful. There is plenty of writing about that, which can
be easily found on the Net.

Yep.

 

[Guest]

[...]

Goto is not dangerous or evil, it's just easy to use in ways that mess up
your code.
[...]

To me it seems people tend to forget why they should not use goto, and
just refrain from using it because their teachers told them not to.

Those teachers probably got their clues directly or indirectly from papers
that analyzed the subject deeply. Saying "goto is not dangerous or evil,"
without refuting the points of those papers doesn't mean much.

You should always consider the solution which creates clear and
understandable code which is easy to maintain.

Those are some of the reasons why goto should not be used:

- goto is not clear: Going to a specific point in the code doesn't say
anything to the reader. Well structured code and well named objects do.

- goto is hard to maintain: You can't insert new objects in to the code
without scanning the entire function to see whether an evil goto may bypass
your object's initialization.

Ali

 

[Neil Cerutti]

Using goto is not nice nor clean. goto is inherently risky and
the code below is obfuscated.

It ended up a buggy mess. And thus, I refuted myself.

 

[=?iso-8859-1?Q?Ali_=C7ehreli?=]

Sure, but for the purpose of *BAILING OUT OF A DEEPLY NESTED LOOP*

for (...)
{
for (...)
{
for (...)
{
...
if (disaster)
goto bailout;
}
}
}
bailout:

What if someone adds an object before bailout label later in development.
That object is not constructed because of the goto call.

Note that what you propose can occur *WITHOUT* goto, as well:

That's because switch/case is some type of lookup and a number of gotos in
disguise. Also, maybe less than goto, but switch/case is frowned upon in C++
as well...

switch (expr)
{
case 1:
Foo foo;
case 2:
// uh oh, if expr == 2, foo is not constructed!
foo.bar();
break;
default:
break;
}

Ali

 

[Neil Cerutti]

That's because switch/case is some type of lookup and a number
of gotos in disguise. Also, maybe less than goto, but
switch/case is frowned upon in C++ as well...

Yep. If your pesky coding standard forbids goto, switch is a
wonderful simulator. (This is based on something similar I saw at
URL:http://thedailywtf.com/ )

for (int x = 1; x < 8192; ++x) {
switch (x) {
case 10:
std::cout << "Neil Cerutti!!!!!!!\n";
break;
case 20:
x = 10;
break;
}
}

This is known as the Commodore 64 BASIC design pattern.

 

[Ron Natalie]

Destruction is only one side of the coin. How about construction:

for (;
{
goto future;
}

Foo foo;

future:

cout << "doomed\n";

Unfortunately foo will not be constructed in the future.

The above program isn't well-formed. The compiler will not let
you transfer control past the initializer.

 

[=?ISO-8859-15?Q?Juli=E1n?= Albo]

Sure, but for the purpose of *BAILING OUT OF A DEEPLY NESTED LOOP*

for (...)
{
for (...)
{
for (...)
{
...
if (disaster)
goto bailout;
}
}
}
bailout:

The better solution is to not write deeply nested loops.