Is this the correct use of a try/catch block

Question

I'm trying to decode URLs that may be single or double percent encoded using decodeURIComponent(). Because I don't want to get an "URI malformed" when a string is attempted to be decoded that only contains %25 (5), I'm using this try/catch block. Have I used the try/catch block correctly? It seems to work, but I'm trying to understand if this is a best practice approach, or if I've over-complicated things.

var uriCleanup = function (str) {

    var cln = function (str) {

        try {

            str = decodeURIComponent(str);
            //try to decode the URI again
            return cln(str);

        } catch (e) {
           //if error thrown, URI contains single percent symbol
           //return the str as it was the last time it was decoded
           return str;

        }

    };

    return cln(str);

};

JSBIN

raina77ow · Accepted Answer · 2014-04-10T18:48:05.903

2

You do overcomplicate things: there's no need to use recursion here.

function uriCleanup(str) {
    var cln = str;
    try {
        do {
            cln = decodeURIComponent(str);
        } while (cln !== str && (str = cln));
    }
    catch (e) {}
    return cln;
}

edited Apr 10 '14 at 18:48

answered Apr 10 '14 at 18:40

raina77ow

103,633
15
192
229

Thanks. I notice that you didn't remove the try/catch block. Do you think that is okay, or is there a way to do without, especially since nothing is being done in the catch statement. – 1252748 Apr 10 '14 at 18:54
Of course it's ok in this particular case; all the other means to prevent that pesky `URIError: malformed URI sequence` are worse by a mile. But in your original code each subsequent invocation of `cln` created their own try-catch scope, which was bad (as it wasn't really needed). – raina77ow Apr 10 '14 at 18:56
what is happening in this evaluation? `cln !== str && (str = cln)`. – 1252748 Apr 10 '14 at 19:04
@thomas First, `cln` (the result of decoding) is compared with the original string (stored in `str`). If they're identical, no further decoding change anything, so the loop breaks. If they're not, the result of decoding is stored in the `str` value, and the loop goes on to process it. – raina77ow Apr 10 '14 at 21:04
Thank you that's clear now. So in JavaScript evaluations you can include variable variable assignments after an `&&`? I was aware that a for-loop of course included this sort of declaration `for (var i = 0,len=myArray.length; i – 1252748 Apr 11 '14 at 13:30
@thomas Assignment is just an expression, so you can use it as part of another expression freely. Just remember that `=` operator's precedence is [one of the lowest](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence), so it's most likely you'll have to wrap assignment in parens - like in that example. – raina77ow Apr 11 '14 at 14:34
how does the `&&` work in that case? Why isn't a semicolon or something. `&&` seems to require that the following expression (the assignment) evaluate to `true`. I mean, yeah: `!!(1+1) //true`, but why is it necessary that `&&` _require_ it resolve to true. Or maybe I've missed something fundamental. Thanks again. – 1252748 Apr 11 '14 at 15:42
It works very similar to comma (sequence) operator, except one thing: `str = cln` will be evaluated only if `cln !== str` is `true`. The result of `str = cln` matters only if it's falsy value, but that's just not possible: if `cln` is an empty string (the only falsy value of String type in JS), `str` could only be an empty string too; hence the first expression is false too. – raina77ow Apr 11 '14 at 21:15

djechlin · Answer 2 · 2014-04-10T18:59:42.020

It's not best practice. try/catch you should use, and only use, for handling exceptional situations that interrupt the flow of code and break your ability to complete it. If you use it more generally, you wind up with a very powerful goto-like structure with unwieldy use in your code.

try/catch is a harder structure to learn compared to if, else, while, etc., because it only really makes sense if you're working in a somewhat large code base, with modular library functions.

A simple example might be the function

var saveOrder = function(order) {
    if(!order.confirmed) {
        throw new Error("Please confirm you order.");
    }
    saveToMySQL(order);
};

//... somewhere else

try { 
    order = getOrderFromUser();
    saveOrder(userOrder);
    alert("Thank you for your order, continue shopping?");
}
catch(err) {
    alert(err);
}

The points here are

saveOrder throws because it cannot proceed
try/catch is only used by the method that believes it can handle the exception
if no exception happens, the code reads linearly and cleanly, and the following line never has to worry about whether the previous line succeeded

This is a really nice explanation, thank you. I think that I don't see any other way to avoid the use of try/catch to avoid that error. If not try/catch, what do you suggest? — 1252748, Apr 10 '14 at 19:09
@thomas You can use try-catch, just only use it to handle exceptional exceptions, not to create clever control flows. raina77ow's answer is good. — djechlin, Apr 10 '14 at 21:06

Is this the correct use of a try/catch block

2 Answers2