3

I'm getting a reference error saying check is not defined when I call check().then(function(employees) towards the bottom of my code and I;m not sure why. This happens when I call getAllEmployees(); 

var employees = []; var departments = []; var error = 0; function initialize(){ var fs = require("fs"); fs.readFile("./data/employees.json", 'utf8', function(err, data){ if(err){ error = 1; } employees = JSON.parse(data); //console.log(employees); }); fs.readFile("./data/department.json", 'utf8', function(err, data){ if(err){ error = 1; } departments = JSON.parse(data); // console.log(departments); }); let check = function(){ return new promise(function(resolve,reject){ if (error === 0){ resolve("Success"); } else if(error === 1){ reject("unable to read file"); } }) }; } function getAllEmployees(){ check().then(function(employees){ console.log(employees); }).catch(function(){ console.log("No results returned"); }); }
Improve this question
0

3 Answers 3

Reset to default
2
  • You forgot to capitalize Promise
  • Your code structure is terrible
  • Wrap legacy node async code in Promises
  • Defer error handling code to caller
  • import native node libs at root level (make them consts too)

Here is the refactored version:

const fs = require("fs"); const util = require("util"); const read = util.promisify(fs.readFile); function init() { var reads = ["employees.json", "department.json"] .map(file => read(`./data/${file}`, "utf8").then(JSON.parse)); return Promise.all(reads) .then(([employees, departments]) => ({ employees, departments })); } init().then(console.log).catch(console.error);
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Great refactored version
I would suggest putting the JSON.parse() in the myread() function to make it more DRY. And, in node version 8+, you can use util.promisfy() instead of manually promisifying it. I would also suggest showing a .catch() to handle errors.
nah, I don't want to imply any specific use of myread(), keepin it generic. If I really wanted to be DRY, I could just var load = ['path1.json', 'path2.json'].map(j => myread(j).then(JSON.parse)); which would incur a negligible runtime overhead.
I added the other suggestions though
Nice use of the spread syntax to clarify what's in [0] and [1] on the array.
|
1

Function check is not defined in the scope of getAllEmployees function, because you have defined check function as local function inside initialize function.

First weird thing:

function initialize(){ var fs = require("fs");

Maybe it's best if you set all require modules at the top, not inside the function.

I did some modifications to your code, now you won't have that error:

var employees = []; var departments = []; var error = 0; var fs = require("fs"); function initialize(){ fs.readFile("./data/employees.json", 'utf8', function(err, data){ if(err){ error = 1; } employees = JSON.parse(data); //console.log(employees); }); fs.readFile("./data/department.json", 'utf8', function(err, data){ if(err){ error = 1; } departments = JSON.parse(data); // console.log(departments); }); } function check() { return new promise(function(resolve,reject){ if (error === 0){ resolve("Success"); } else if(error === 1){ reject("unable to read file"); } }) }; function getAllEmployees(){ initialize(); check().then(function(employees){ console.log(employees); }).catch(function(){ console.log("No results returned"); }); }

Summary of changes I did:

First, locate the main problem about you are asking, why are you getting "check function is not found"?. As I said in the beginning, check function was declared in the initialize scope function and getAllEmployees was declared outside so the main error is clear.

What I did: change initialize function to only get json data and set two global variables: employees and departments. Move check function to outside of initialize scope, as a module function to check promises.

Change getAllEmployees function to first, call initialize (to get json data and set the global variables) and call check function.

There are best ways to manage promises as for example the way provide as @Rafael in his answer.

Improve this answer

3 Comments

It works but printing the employees array results inside getAllEmployees results in an empty array
Maybe you have a different error due to what are you getting from json, maybe you should ask a new question with that problem
I will ask a new question. Thanks for your help
1

The explicit construction is an antipattern. If you have Node.js 8 you can use promisify or use bluebird version of with an older environment. With it you can rewrite your code in much cleaner way and handle the errors properly.

Also it make sense to say that its a bad practice to throw(reject with) string

const {promisify} = require('util'); const fs = require('fs'); const readFileAsync = promisify(fs.readFile); Promise.all([ readFileAsync("./data/employees.json", {encoding: 'utf8'}).then(x => JSON.parse(x)), readFileAsync("./data/department.json", {encoding: 'utf8'}).then(x => JSON.parse(x)) ]) .then(([employees, departments]) => { console.log(employees); }) .catch(console.log)
Improve this answer

4 Comments

FYI, readFileAsync() does not automatically parse the JSON data.
Well, your code doesn't do what the OP's code does without the JSON.parse(). It looks simpler than it really can be. Why don't you just edit and fix?
Well stackoverflow isn't about rewriting somebody else code. IMHO, it's about ideas. But... edited.
Your choice what kind of answers you want to supply, but complete answers that show everything required to solve the OP's problem are generally considered better answers which is why I commented. FYI, .then(x => JSON.parse(x)) can be .then(JSON.parse).

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.