CODEWITHSUNDEEP
branchif-statement
Daan
Daan

Reputation: 6994

How to handle an invalid execution state?

Say you have the following code block:

if (Light.On) {
    // do something...
} else if (Light.Off) {
    // do something else...
} else {
    // this state should never be reached
}

Now assume that the applications logic dictates that in this part of the code, the last state should never be reached, but it is not something that can be determined at compile time. And maybe there ARE other states (e.g. Light.Broken) which could be set by other parts of the application, but which are not used here.

What code do you add in the last else block?

  1. Add no code, because it should not be reached anyway.
  2. Add some logging functionality, so that you as a developer know that some illegal state has been reached.
  3. Throw an exception, because the state must not be reached and if it is reached anyway, something else must be wrong.

The first option doesn't seem reasonable to me, hoping something goes right seems hardly the right choice. Option two has the advantage that your app doesn't crash right away, so if this happens in a rare occasion which was not caught in testing, the customer can continu using the application and the developer is notified of the problem. Option three causes the app to crash, which obviously is not something you want your customers to experience, but it does make it very clear that something is wrong.

What is the best way to handle such a situation?

EDIT, based on comments:

Some additional consideration to steer the discussion:

Upvotes: 0

Views: 907

Answers (7)

Adrian Grigore
Adrian Grigore

Reputation: 33318

In the development version, just throw an exception.

In the release version:

  • Throw an exception
  • Save the user's work in some temporary file if possible
  • Save a crashdump and (using a small helper application) offer the user the option of sending it to you so you can identify and fix the problem

Upvotes: 0

mouviciel
mouviciel

Reputation: 67859

As already said, in development phase you should make it visible as soon as possible. In release phase, it depends on how critical it is to reach this invalid state.

The least is to issue a log for debugging purpose.

Then you can try to recover from this invalid state either by going back to a previous valid state or by going to a new valid state.

Finally if nothing safe can be done, you can terminate the execution (with an alert to the user and a log to the maintainer).

Upvotes: 1

pauljwilliams
pauljwilliams

Reputation: 19225

In development - fail hard and fail fast. Throw some kind of runtime exception, or just Assert(false).

In a release - shutdown gracefully. Your app is in an unusable state, and you cant realy on anything that you normally would, such as class invariants etc. Give the user a chance to save thir work, for example, try and log an error that could be sent back to the dev team and then shutdown.

EDIT: Based on comments added.

If the contract of the function states that the light shouold be on or off at function entry then any other state is an error. The function should fail, according to the principles outlined in my original answer.

Regarding the 'non-critical' aspaect - if a function precondition is not being met that means that your application is broken. Regardless of wether the error is detected in a non-critical piece of code, that does not imply that the problem itself is non-critical - you have no way of knowing that the bug that creates the invalid state doesnt also affect critical areas of code.

Upvotes: 6

eglasius
eglasius

Reputation: 36035

Throw an exception.

As you said, the third case should never occur. If it does happens, then something went wrong, and you don't know what else can be going wrong.

You also don't want calling code to think that code is working normally, when it is actually failing.

Additionally it makes it a lot easier to spot the issue Before it is released.

Upvotes: 0

paxdiablo
paxdiablo

Reputation: 882156

Don't do anything. If all you care about is whether the light is on or not:

if (Light.On) {
    // do some work
} else {
    // too darned dark to work
}

Otherwise you should enumerate all your states.

If the other states are not allowed, you should error. If they're allowed but irrelevant, just ignore them. With properly designed code, there is no problem.

You're main problem here lies in a design that can have Light.On and Light.Off set at the same time. You should be using a state Light.State which is set to one of {on, flickering, off, broken, unplugged, exploded, emitting_dangerous_gamma_rays} and so on.

Upvotes: 0

unwind
unwind

Reputation: 400019

Well ... It depends. Having a third case for a boolean test would make me want to cry. It's just noise, adding confusion and telling me the developer was at least as confused as it makes me feel.

For a non-boolean, I guess you could do whatever ... If the ifs capture the states that are relevant to the code in question, there's no harm in just ignoring other cases. If there's a good chance that further cases will be needed in the future, a comment might be enough to indicate that for clarity.

Upvotes: 2

Bhushan Bhangale
Bhushan Bhangale

Reputation: 10987

3rd option is the correct as if for that part any other state is not valid so it should throw and exception saying invalid state.

Upvotes: 0

Related Questions