Avoiding an empty catch clause

Question

I have a function which checks to see if the user has made any changes, and if so, warns them of this fact. When they choose to discard their changes, I have another function which a) restores the state to pre-edit, and b) updates a global object which holds info about the current edit (including whether one exists).

What I don't want to happen is that some error is thrown on trying to remove the edit box elements, and as a result the system doesn't update the global object flag or show the hidden pre-edit elements. If this happens, the program may think an edit is still going on when it isn't, leaving the user trapped in a "Discard changes?" cycle. For this reason, I catch any errors thrown in the destroy phase, and then show the hidden elements and update the global, like this:

function cancelEdit() {

    try {
        // destroy editing boxes
        // [code goes here]
    } catch(e) {
    } finally {
        // restore hidden elements
        // [code goes here]

        // update global edit cache object
        // [code goes here]

        // rethrow the error for analysis server-side
        if(window.onerror) window.onerror();
    }

}

Having an empty catch block like above seems like a code smell to me, but I don't think this way is necessarily better. (But maybe it is.)

function cancelEdit() {

    try {
        // destroy editing boxes
        // [code goes here]
    } catch(e) {
        cancelEditInternal();
        // rethrow the error for analysis server-side
        throw e;
    }

    cancelEditInternal();

}

function cancelEditInternal() {

    // restore hidden elements
    // [code goes here]

    // update global edit cache object
    // [code goes here]

}

Am I missing something? Is there a pattern that I'm neglecting... or is this just a consequence of the fact that I'm using try/catch/finally where it isn't usually used?

Whoops, I had a feeling I was overthinking it. Thanks for the quick responses. — Chris Middleton, Oct 23 '14 at 20:53

score 4 · Accepted Answer · answered Oct 23 '14 at 20:46

4

You can use a finally block:

function cancelEdit() {
    try {
        // destroy editing boxes
        // [code goes here]
    } finally {
        cancelEditInternal();
    }

}

The finally block will be executed whether or not the body of the try throws an error. (If you keep the catch clause, the finally block will still execute.)

answered Oct 23 '14 at 20:46

Ted Hopp

232,168
48
399
521

I realized after seeing these responses that my model up above is flawed, since when `window.onerror` is called, it doesn't receive any arguments. Will a window.onerror get called if I don't have the catch block? (Or maybe even with a catch block?) – Chris Middleton Oct 23 '14 at 20:51
@AmadeusDrZaius - `window.onerror` will be called if the script generates an error. If you catch the error within the script, then neither `window.onerror` nor the default event handler will be called (unless you rethrow the error). – Ted Hopp Oct 23 '14 at 20:54

score 3 · Answer 2 · answered Oct 23 '14 at 20:46

If you don't want the catch block, then don't use it:

try {
    // destroy editing boxes
    // [code goes here]
} finally {
    // restore hidden elements
    // [code goes here]

    // update global edit cache object
    // [code goes here]

    // rethrow the error for analysis server-side
    if(window.onerror) window.onerror();
}

As you can see in the specification, a try statement consist of try followed by either catch or finally or both.

Avoiding an empty catch clause

2 Answers2

Linked