Is it a good practice to add extra checks in disabled GUI options?

Question

I'm referring to the following:

void setup_gui()
{
    if (some_condition)
        some_button.disable();

    ...
}

void some_button_click()
{
    // Is this a good practice?
    if (some_condition)
        return;

   ...
}

Adding that check ensures that the program won't run the operation, but it can also be seen as hiding a bug (some_button_click() must not have run at all).

So, what do you think about it? Is it a safe coding practice or hiding a bug?

Andy Thomas · Answer 1 · 2011-01-05T16:10:45.643

2

Defensive programming is as reasonable as defensive driving.

It may be helpful to think of this in terms of separate concerns. One concern is the presentation. Another may be a set of business rules. It is reasonable to make the same check in both places.

You want to make the check in the presentation layer to communicate to the user.

You may also want to make the check below the presentation layer:

To defend against present and future mistakes in the presentation layer.
In case the code underneath the presentation layer is re-used elsewhere.
[From mvds's comment] In case the condition may change since the control was enabled or disabled.

Edit: David Heffernan's DRY concern below can be addressed trivially by defining the condition exactly once, and accessing it elsewhere.

void setup_gui()
{
    some_button.setEnabled( context.isThisActionAvailable() );

    ...
}

void some_button_click()
{
    if ( context.isThisActionAvailable() )
        return;

   ...
}

edited Jan 05 '11 at 16:10

answered Jan 05 '11 at 14:55

Andy Thomas

84,978
11
107
151

1

+1 for this realistic answer. Consider web design, where you may hide some control: in that case it is perfectly logical to check again when the action is performed, because the server/session state may have changed (for whatever reason). In standalone application development the examples are not so trivial, but you may not always be fully confident that every conceivable state transition will be caught by your code, and that in some rare corner case the button is enabled accidentally. – mvds Jan 05 '11 at 15:53
I have to add to that: defensive coding would also mean that the stored state (in Nib, layout xml, or other means of storing GUI) has everything disabled by default, and that `setup_gui()` only *enables* elements. – mvds Jan 05 '11 at 15:55
This is better but I still think that the best solution is to have a GUI framework that works. Think of it as a GUI framework rather than a GUI frame-not-work!! – David Heffernan Jan 05 '11 at 16:08
@David Heffernan - You and I may be thinking of this differently. If some_button_click() were an event handler tightly coupled with the button, then an additional check might be unnecessary but harmless (though see mvds's comment). On the other hand, if it is not tightly coupled, or if it may have multiple potential triggers present and future, then the check reduces risk. Reducing risk is a good thing. – Andy Thomas Jan 05 '11 at 23:03

score 1 · Answer 2 · answered Jan 05 '11 at 15:09

I don't take this belt and braces approach. The problem is that you have violated the DRY principle with the double use of some_condition. It's all to easy to change this in one place and not the other.

Of course some_condition is quite simple in this made-up example but in reality it's often be much more complex.

If you can't trust your GUI framework to block actions when you request them to be blocked, then you need to fix the framework.

score 0 · Answer 3 · answered Jan 05 '11 at 15:03

It can be considered safe coding, it can also be considered hiding a bug. This is the time when perhaps you should re-evaluate your program logic. Code redundancy is something that is always best to avoid if possible. It would be better if your program could be structured in such a way that it would not need to check twice, by making sure the actual logic of the program works properly and as intended.

Sure, if you're in a rush this is a quick fix and it works, but it reeks of poor design and/or logic elsewhere in the program.

score 0 · Answer 4 · answered Jan 05 '11 at 15:13

0

The silent return in some_button_click() potentially hides a bug. I would either don't do the check at all, or crash loud and violently.

answered Jan 05 '11 at 15:13

Ronnis

12,593
2
32
52

Is it a good practice to add extra checks in disabled GUI options?

4 Answers4