Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error on module part of learnyounode workshop #2068

Closed
vikmn opened this issue Apr 11, 2017 · 8 comments
Closed

Error on module part of learnyounode workshop #2068

vikmn opened this issue Apr 11, 2017 · 8 comments
Labels
learnyounode

Comments

@vikmn
Copy link

vikmn commented Apr 11, 2017

Working on the modular part of the learnyounode workshop, i get the following error however all the validation steps pass. So i am not sure if the error is in my code or if the validator is failing for some other reason? Appreciate any feedback on what the issue is in this instance.

If you have a problem with an error message:

  • node version: v6.9.5
  • npm version: 4.4.4
  • os:windows 10

Error output

 learnyounode verify filtered-ls.js

# LEARN YOU THE NODE.JS FOR MUCH WIN!

## MAKE IT MODULAR (Exercise 6 of 13)


Your submission results compared to the expected:

                 ACTUAL                                 EXPECTED
────────────────────────────────────────────────────────────────────────────────

   "CHANGELOG.md"                      ==    "CHANGELOG.md"
   "LICENCE.md"                        ==    "LICENCE.md"
   "README.md"                         ==    "README.md"
   ""                                  ==    ""

────────────────────────────────────────────────────────────────────────────────

 ✓

 Submission results match expected

 ✓

 Additional module file exports a single function

 ✓

 Additional module file exports a function that takes 4 arguments

 ✓

 Additional module file handles errors properly

 ✓

 Additional module file handles callback argument

c:\code\node-workshop\directory-utils.js:12
            callback(fileName);
            ^

TypeError: callback is not a function
    at c:\code\node-workshop\directory-utils.js:12:13
    at Array.map (native)
    at c:\code\node-workshop\directory-utils.js:11:12
    at FSReqWrap.oncomplete (fs.js:123:15)

This is my filtered-ls.js

 const directoryPath = process.argv[2];
const fileExtension = process.argv[3];
var directory = require('./directory-utils.js')
var logger = require('./console-logger.js');
var logError = function (error) {
    console.error(error);
};
directory(directoryPath, fileExtension, logger.logError, logger.logInfo);

and this is my directory-utils.js

var directoryReader = require("fs");
var pathReader = require("path");

module.exports = function filter(directoryPath, fileExtension, errorCallback, callback) {

    directoryReader.readdir(directoryPath, function (error, files) {
        if (error)
            return errorCallback(error);
        files.filter(function (fileName) {
            return pathReader.extname(fileName).slice(1) === fileExtension;
        }).map(function (fileName) {
            callback(fileName);
        });
    });
};
@francesmcmullin
Copy link

francesmcmullin commented Apr 11, 2017 •
edited
Loading

Verification is failing on the step "Additional module file handles callback argument" - so it's testing your module file directory-utils.js. It seems that, although this was not thoroughly spelled out in the exercise description, the verification step requires that your function accepts just three arguments - the third one being callback which can be called with an error first parameter, or with data in the second parameter.

With your module expecting callback as the fourth parameter and the verification passing in only a single callback function in the third position, the callback parameter is undefined, which is indeed not a function.

If you refactor your module to use a single callback parameter, similar to how fs.readdir works, all steps should pass.

@vikmn
Copy link
Author

vikmn commented Apr 12, 2017 •
edited
Loading

Having modified my directory utils as mentioned above to just use 3 arguments

function (directoryPath, fileExtension, cbLogger) {

    directoryReader.readdir(directoryPath, function (error, files) {
        if (!error)
            return files.filter(function (fileName) {
                return pathReader.extname(fileName).slice(1) === fileExtension;
            }).map(function (fileName) {
                cbLogger(null, fileName);
            });

        cbLogger(error);
    });
};

I still get a variant of the same error

node_modules\learnyounode\node_modules\workshopper-exercise\exercise.js:188
    processors[i].call(self, mode, function (err, pass) {
                 ^

TypeError: Cannot read property 'call' of undefined
    at next (\node_modules\learnyounode\node_modules\workshopper-exercise\exercise.js:188:18)
    at \npm\node_modules\learnyounode\node_modules\workshopper-exercise\exercise.js:195:7
    at callback (make_it_modular\verify.js:26:15)
    at modFileError (make_it_modular\verify.js:31:5)
    at \make_it_modular\verify.js:131:18
    at c:\code\node-workshop\directory-utils.js:11:17
    at Array.map (native)
    at c:\code\node-workshop\directory-utils.js:10:16
    at FSReqWrap.oncomplete (fs.js:123:15)

My caller does look like this

var directory = require('./directory-utils.js')
var logInfo = function (error, fileName) {
    if (error)
        return console.error(error);
    console.log(fileName);
};
directory(directoryPath, fileExtension, logInfo);

@francesmcmullin
Copy link

francesmcmullin commented Apr 12, 2017 via email

@vikmn
Copy link
Author

vikmn commented Apr 12, 2017

Thanks modifying th code to return the files rather than calling the logger within the map function helped and got me past the verification too albeit i did avoid the use of a forech loop as per the suggested by the final solution. Just wondering if the notes on the verification steps could be made a bit more clear to highlight this situaion? Otherewise happe for this to be closed.

@gyoungdahl
Copy link

gyoungdahl commented Apr 13, 2017 •
edited
Loading

Hi vikmn,

Your current issue (the analysis program failing because it is trying to access a property of an undefined

array entry - processors[i].call) is the same one I have been discussing in the topic titled "Error when
verifying my program (apparently unrelated to my program)". In fact I was trying the same project
("Make It Modular") and making the exact same mistake (calling the callback multiple times - once with
each matching file I detected). It took me over two weeks to get attention to it, probably because I didn't
have the best topic description. I got the Make It Modular exercise to complete by changing my code to
only call the callback once passing an array of all the filenames.

However I might propose that the program that analyzes a student's code for adherence to the

exercises ought to be more robust than to crash in the face of what it seems is a common
misinterpretation of the exercise (both you and I made the same assumption - I'd bet there would be
more). While the repercussion of the failure of the analysis program isn't as severe as say a failure in the
program controlling a nuclear reactor could be, this is a classic example of a program that ought to be
pretty fault tolerant - its job is to analyze the student's implementation and find the faults with it. So it
ought not crash when a student's code has a fault. My first implementation ran fine if I launched it
directly under the "node" application, and also if I "ran" it in the learnyounode application. Only when I
tried to "verify" it under learnyounode did the crash occur. So it really seemed like the analysis code had a
bug, especially because my application was working (producing the expected output and not crashing)
when run without the analysis code looking at it.

One might think that whoever develops these training materials might be interested in addressing this

now common (?) issue with that exercise (actually with the code that analyzes the potential solution) to
check for an undefined array entry before trying to reference a property of it. If someone knows how
such an issue might be reported here, that might be a good approach to follow for this situation.

--
AETLM

[EDIT: To add some newlines - even though it looked fine when I entered it, each paragraph was one long
line that didn't wrap when I viewed it from the forum - at least in my browser.]

@francesmcmullin
Copy link

francesmcmullin commented Apr 14, 2017 •
edited
Loading

I tried reproducing this variety of problem on my machine and I saw output like this:

Your submission results compared to the expected:

                 ACTUAL                                  EXPECTED                 
────────────────────────────────────────────────────────────────────────────────

   "CHANGELOG.md"                      ==    "CHANGELOG.md"                     
   "LICENCE.md"                        ==    "LICENCE.md"                       
   "README.md"                         ==    "README.md"                        
   ""                                  ==    ""                                 

────────────────────────────────────────────────────────────────────────────────

✓ Submission results match expected
✓ Additional module file exports a single function
✓ Additional module file exports a function that takes 3 arguments
✓ Additional module file handles errors properly
✓ Additional module file handles callback argument
✓ Additional module file returned two arguments on the callback function
✗ Your additional module file [mod.js] did not return an Array object as the second argument of the callback
✓ Additional module file returned two arguments on the callback function
✗ Your additional module file [mod.js] did not return an Array object as the second argument of the callback
/usr/lib/node_modules/learnyounode/node_modules/workshopper-exercise/exercise.js:152
    processors[i].call(self, mode, function (err, pass) {
                 ^

TypeError: Cannot read property 'call' of undefined
    at next (/usr/lib/node_modules/learnyounode/node_modules/workshopper-exercise/exercise.js:152:18)
    at /usr/lib/node_modules/learnyounode/node_modules/workshopper-exercise/exercise.js:159:7
    at callback (/usr/lib/node_modules/learnyounode/exercises/make_it_modular/verify.js:25:15)
    at modFileError (/usr/lib/node_modules/learnyounode/exercises/make_it_modular/verify.js:30:5)
    at /usr/lib/node_modules/learnyounode/exercises/make_it_modular/verify.js:120:18
    at /home/cocoa/mod.js:11:17
    at Array.map (native)
    at /home/cocoa/mod.js:10:16
    at FSReqWrap.oncomplete (fs.js:112:15)

While I agree that the error text is not helpful, the verification messages stating "Your additional module file [mod.js] did not return an Array object as the second argument of the callback" are attempting to point the student in the right direction. Can I ask if either of you saw that message? If not, I'd be worried that this verification is not catching the case it was intended for.

As far as errors from the exercise verification and bugs go, I can only point you to learnyounode's issue tracker, which already has at least one issue relating to this scenario. I agree it would be wonderful for the exercises to be robust and error free, but like most open source projects learnyounode is maintained entirely by volunteers and is always in need of more help. If you find you have the time and the energy to contribute and improve the learning experience for others, then I'm sure your efforts would be appreciated.

[edited to add] I am not a maintainer or contributor to learnyounode. I very occasionally read issues on this repository for a few workshop's I'm familiar with and try to help out, but my time and energy are quite limited. I do not want to blame anyone for expecting a better learning experience, learning is difficult and our tools and support can always get better, but I do want to balance expectations against the voluntary nature of these projects.

@gyoungdahl
Copy link

gyoungdahl commented Apr 14, 2017 via email

@vikmn vikmn closed this as completed Apr 20, 2017
@zzy1136660162
Copy link

You should execute the callback function only once in the "mymodule" file
Example:

mymodule.js

const fs = require("fs");

module.exports = function (path, ext, callback) {
  fs.readdir(path, function (err, files) {
    if (err) callback(err, null);
    try {
      callback(
        null,
        files
          .filter((s) => s.indexOf(ext || "") != -1)
          .filter((s) => s && s.indexOf(".") != -1)
      );
    } catch (error) {
      callback(err, null);
    }
  });
};

make-it-modular.js:

const mymodule = require("./mymodule");
mymodule(process.argv[2], process.argv[3], function (err, data) {
  data.forEach((element) => {
    console.log(element);
  });
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
learnyounode
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@francesmcmullin @riyadhalnur @vikmn @gyoungdahl @zzy1136660162