If vs try/catch

Question

The following code, written in synchronous Node.js and intended to be used as a shell script, nests a try/catch inside a conditional.

Because both the conditional and the try/catch serve a similar purpose - respectively, check a file exists, and check that reading and writing to it succeeds - it feels clumsy that the code uses two different mechanisms. The nesting may also be unnecessary.

What would be a more readable way to write this code?

#!/usr/bin/env node

var fs = require('fs');

var path = process.argv[2];
var data = "#!/usr/bin/env node\n\n";
if (fs.existsSync(path)) {
  try {
    data += fs.readFileSync(path);
    fs.writeFileSync(path, data);
  } catch (err) {
    console.log("Problem reading or writing " + path + " : " + err.message)
    process.exit(1);
  }
} else {
  console.log("Invalid path");
  process.exit(1);
}

Answer 1

In general, a good strategy is to return early as soon as you detect an error. Putting the error handler for fs.existSync(path) in an else block far away from where the check occurred makes the code harder to follow, and increases the amount of nesting required.

Also, the error message should inform the user of which path is invalid.

#!/usr/bin/env node

var fs = require('fs');

var path = process.argv[2];
var data = "#!/usr/bin/env node\n\n";
if (!fs.existsSync(path)) {
    console.log("Invalid path: " + path);
    process.exit(1);
}
try {
    data += fs.readFileSync(path);
    fs.writeFileSync(path, data);
} catch (err) {
    console.log("Problem reading or writing " + path + " : " + err.message)
    process.exit(1);
}