debugjavascriptMinor
If vs try/catch
Viewed 0 times
catchtrystackoverflow
Problem
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?
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);
}Solution
In general, a good strategy is to return early as soon as you detect an error. Putting the error handler for
Also, the error message should inform the user of which path is invalid.
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);
}Code Snippets
#!/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);
}Context
StackExchange Code Review Q#59964, answer score: 2
Revisions (0)
No revisions yet.