Other way to write this?

[Alex]

This is a question of style I guess..
I have something like this:

if (ShowDialog()) {
if (Dialog->ShowMessage(...) == KEY_OK) // User pressed ok button
PerformAction();
}
else
PerformAction();

--

The idea is that if ShowDialog() is true, then the program shows a dialog
with some info before proceeding. Otherwise, the action is performed
immediately.
What I don't like is repeating the PerformAction() sentence.. is there a way
to avoid this more 'elegantly'? I mean, refactoring in a way to make
PerformAction() appear only once, regardless the ShowDialog() return value?

Thanks in advance.

 

[Andre Kostur]

This is a question of style I guess..
I have something like this:

if (ShowDialog()) {
if (Dialog->ShowMessage(...) == KEY_OK) // User pressed ok button
PerformAction();
}
else
PerformAction();

--

The idea is that if ShowDialog() is true, then the program shows a
dialog with some info before proceeding. Otherwise, the action is
performed immediately.
What I don't like is repeating the PerformAction() sentence.. is there
a way to avoid this more 'elegantly'? I mean, refactoring in a way to
make PerformAction() appear only once, regardless the ShowDialog()
return value?

if (!ShowDialog() || (Dialog->ShowMessage(...) == KEY_OK))
{
PerformAction();
}


Time to brush up on your boolean logic...

 

[Victor Bazarov]

This is a question of style I guess..
I have something like this:

if (ShowDialog()) {
if (Dialog->ShowMessage(...) == KEY_OK) // User pressed ok button
PerformAction();
}
else
PerformAction();

Maybe something like

if (!ShowDialog() || Dialog->ShowMessage() == KEY_OK)
PerformAction();

V

 

[Alf P. Steinbach]

* Alex:

This is a question of style I guess..
I have something like this:

if (ShowDialog()) {
if (Dialog->ShowMessage(...) == KEY_OK) // User pressed ok button
PerformAction();
}
else
PerformAction();

The idea is that if ShowDialog() is true, then the program shows a dialog
with some info before proceeding. Otherwise, the action is performed
immediately.
What I don't like is repeating the PerformAction() sentence..

What I don't like is the use of conditions with side-effects.

That is, incidentally, what makes it hard to rewrite.

is there a way
to avoid this more 'elegantly'? I mean, refactoring in a way to make
PerformAction() appear only once, regardless the ShowDialog() return value?

if( !ShowDialog() || ShowMassageOK() ) { PerformAction(); }

Thanks in advance.

T'was nothi
n
g...

 

[Kai-Uwe Bux]

This is a question of style I guess..
I have something like this:

if (ShowDialog()) {
if (Dialog->ShowMessage(...) == KEY_OK) // User pressed ok button
PerformAction();
}
else
PerformAction();

--

The idea is that if ShowDialog() is true, then the program shows a dialog
with some info before proceeding. Otherwise, the action is performed
immediately.
What I don't like is repeating the PerformAction() sentence.. is there a
way to avoid this more 'elegantly'? I mean, refactoring in a way to make
PerformAction() appear only once, regardless the ShowDialog() return
value?

Thanks in advance.

What about:

if ( // we perform the action if
( ! ShowDialog() ) // the message is skipped
|| // or
(Dialog->ShowMessage(...) == KEY_OK) // the user accepts
){
PerformAction();
}


Kai-Uwe Bux

 

[Jules]

A different solution to the suggested ones, and not applicable in as
many situations, but you may want to consider it:

class CancelOperation { };

try
{
if (ShowDialog() && Dialog->ShowMessage(...) != KEY_OK)
throw CancelOperation;
PerformAction();
}
catch (CancelOperation e)
{
}

 

[Victor Bazarov]

A different solution to the suggested ones, and not applicable in as
many situations, but you may want to consider it:

class CancelOperation { };

try
{
if (ShowDialog() && Dialog->ShowMessage(...) != KEY_OK)
throw CancelOperation;
PerformAction();
}
catch (CancelOperation e)
{
}

Could you (for everybody's benefit) show in what conditions your
solution would be superior to other ones? Thanks.

V

 

[James Aguilar]

if (ShowDialog()) {
if (Dialog->ShowMessage(...) == KEY_OK) // User pressed ok button
PerformAction();
}
else
PerformAction();

if (!(ShowDialog() && Dialog->ShowMessage(...) != KEY_OK))
PerformAction();

This doesn't repeat the PerformAction call, but it is also less clear.

JFA1