Separate logic in files node

Question

I have a Node project and am trying to make my code better, so I've created a new folder with a file like following which encapsulates the read and write file.

This is working and my question is if it's good practice since I need to use the call to read and write file in several place in my code.

var fs = require('fs');

function writeFile(filePath, fileContent) {
    fs.writeFile(filePath, fileContent, 'utf8', function (err) {
        if (err) {
            return console.log("error to write to file:  " + filePath + " " + err);
        }
    });
}
function readFile(filePath, content) {
    fs.readFile(filePath, 'utf8', function (err, data) {
        if (err) {
            return console.log("error to read file:  " + filePath + " " + err);
        }
        content(data);
    });
}
exports.readFile = readFile;
exports.writeFile = writeFile; 

I call it in my modules like this:

module.exports = function (app) {
    app.get('/aa', function(req, res) {

        var filePath  = 'C://test.txt';
        var file = require('../helprs/file');
         file.readFile(filePath ,function(fileContent){
            console.log(fileContent);
            res.send(fileContent);
        });

    });

Answer 1

There are problems with your readFile and writeFile functions. These functions are asynchronous. As such, the callbacks you pass to fs.writeFile() and to fs.readFile() are called some time in the future, long after fs.readFile() and fs.writeFile() have already returned and finished executing.

As such, the return value you have in those callbacks just returns back into the bowels of the file I/O sub-system and does not go back to any of your code.

So, as you have the code written, you have these problems:

1. There's no way for the code that calls readFile() or writefile() to know when either function has completed.

2. There's no way for the code that calls writeFile() to know when that function succeeded or failed.

3. There's no way for the code that calls readFile() to know if an error occured.

Your functions will need to accept either a completion callback or they will need to return a promise because that's how the caller gets completion and error information.

---

Then, once you've fixed the above issues, you'll find that you've hardly changed fs.readFile() and fs.writeFile() much at all. Pretty much all you will have done is default the encoding to utf8 since everythning will be pretty much the same. I'd have to say that this module doesn't really seem worth it.

---

You could use the native fs.readFile() like this:

    var fs = require("fs");

    app.get('/aa', function(req, res) {

        var filePath  = 'C://test.txt';
        fs.readFile(filePath, 'utf8', function(err, data) {
            if (err) {
                res.send(err);
            } else {
                console.log(data);
                res.send(data);
            }
        });

    });

So, this is pretty much the same amount of code as what you had, the main difference being that this version has full error handling whereas your version did not handle errors from fs.readFile() and the browser request was left permanently with no response upon error.

---

Creating modules to contain shared code is a generally good idea. But, when working with async functions and their responses, you need to make sure you preserve full async response and error handling when you write cover functions that use async library functions. In this case, you missed some of that, rendering your cover function not fully functional.

In addition, when covering a built-in function, you should pretty much make sure you're adding enough value to justify a new interface. Simple defaulting one argument is probably not enough change/improvement to justify creating a whole new set of functions that someone learning or modifying your code would have to become familiar with in order to be productive in your code (which would not be an issue if you just used the standard functions).

---

Also, in this specific example, Express has res.sendFile() which will do all this work for you:

    app.get('/aa', function(req, res) {
        res.sendFile('C://test.txt', function(err) {
            if (err) {
                res.status(err.status).end();
            }
        });
    });