Programming standards & refactoring a try/catch statement

Question

Programming standards & refactoring a try/catch statement

Using:

node.js

Problem:

While refactoring my code today I saw these chunks of lines. The interesting thing about them is that each alternative line in the try catch can cause and exception and hence the code can crash. For what practice I follow, each individual exception should be handled specifically and minimum (useful) lines of code should be in the try catch. As a programmer which approach should I be following (A , B , anyother)? And why? I personally would go with A as it decreases the number of lines in code.

Code A:

function(err, someData, res){
    if(err){
        console.error(err);
    } else{

        try{
            data = JSON.parse(someData); //exception expected

            if(someVariable == "abc"){
                var items = data['a']; //exception expected
            } else if(site == "xyz"){
                var items = data['b']; //exception expected
            }

        } catch(err){
            console.error(err);
        }
}

Code B:

function(err, someData, res){
    if(err){
        console.error(err);
    } else{

        try{
            data = JSON.parse(someData); //exception expected
        } catch(err){
            console.error(err);
        }

        if(someVariable == "abc"){
            try{
                var items = data['a']; //exception expected
            } catch(err){
                console.error(err);
            }
        } else if(site == "xyz"){
            try{
                var items = data['b']; //exception expected
            } catch(err){
                console.error(err);
        }
            }
}

javascript
node.js
refactoring

Answer 1

A is fine.

B is bad because say you error on the json parsing, it will continue running the rest of the code even though you know it cannot possibly succeed. Worse still as more code gets added that could actually have really odd side effects or hard to track down bugs. Also it's extremely confusing to read.

You should be grouping your error catching into logical blocks, if one part fails and it makes no sense to run the rest of it, treat it as one block of errors to catch.

Answer 2

I don't think using many try-catch blocks (B) - one for every source code line - has any advantage. Use a single try-catch block (A) for every semantically specific part of code, usually a single method or code block, which must be executed atomically - it should either succeed or fail as a whole.

If you need to process different exceptions in different manner check them in your catch block:

try
{
  // ...
}
catch(e)
{
  if(e instanceof SpecificError)
  {
    //
  }
}

Also you may possibly analyze e.message.

Answer 3

A with a Twist

In "Clean Code", "Uncle Bob" Martin holds that try/catching exceptions is a specific task worthy of its own function. Extract the real work into a separate method. This silly suggestion made me rant at first, but I have grown to actually like the idea.

parseData = function(someData, res) {
    data = JSON.parse(someData); //exception expected

    if(someVariable == "abc"){
        var items = data['a']; //exception expected
    } else if(site == "xyz"){
        var items = data['b']; //exception expected
    }
}

tryParseData = function(err, someData, res){
    if(err) {
        console.error(err);
    } else {
        try {
            parseData(someData, res);
        } catch(err) {
            console.error(err);
        }
    }
}