Confusing error message for 'make it modular'

[mvolz]

Working code was failing with a very confusing error message.

Code failed with error message:

             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 ✓ Additional module file returned correct number of elements as the second argument of the callback ✗ Your additional module file [printfiles.js] did not return an Array with the correct number of elements as the second argument of the callback

Looking at the last two messages, the former seems to contradict directly with the latter, and was uninformative about what the actual issue was.

This issue was the following:

This was the failing program being run:

Program.js: Failing

var printfiles = require('./printfiles.js');

var dirname = process.argv[2];
var filter = '.'+process.argv[3];

printfiles(dirname, filter, function(err, list){
    if (!err) {
        list.forEach(function(i){
            console.log(i);
        });
    }
});

Removing the '.' from the filter variable fixed it, so the program was then:

Program.js: Passing

var printfiles = require('./printfiles.js');

var dirname = process.argv[2];
var filter = process.argv[3];

printfiles(dirname, filter, function(err, list){
    if (!err) {
        list.forEach(function(i){
            console.log(i);
        });
    }
});

And then adding the period to the filter in the imported file instead, made the tests pass.

I'm not sure if either the error message should be fixed to be more informative (because the message before it seemed to directly contradict it, and also was wrong) or if the answer should be accepted as correct; another way to fix this is to simply to specify in the directions that the argument to the imported file should not already include the period.

Rest of code:

printfiles.js, Failing

var fs = require('fs');
var path = require('path');

function printfiles(dirname, filter, callback){
    fs.readdir(dirname, function(err, list){
        var fullList = [];
        if (err){
            return callback(err);
        }
        for (var i in list){
            if (path.extname(list[i]) === filter){
                fullList.push(list[i]);
            }
        }
        callback(null, fullList);
    });
}

module.exports = printfiles;

Passing

var fs = require('fs');
var path = require('path');

function printfiles(dirname, filter, callback){
    filter = '.'+ filter;
    fs.readdir(dirname, function(err, list){
        var fullList = [];
        if (err){
            return callback(err);
        }
        for (var i in list){
            if (path.extname(list[i]) === filter){
                fullList.push(list[i]);
            }
        }
        callback(null, fullList);
    });
}

module.exports = printfiles;

I'm still not exactly sure what the last message refers to; is it possible referring to the length of filter argument, which should have been 2 elements (md) but was instead 3 (.md?) In which case the message should not say "Your additional module file [printfiles.js] did not return an Array with the correct number of elements as the second argument of the callback" which was not the case, but instead refer to the type/length (String/2) of the second argument of the function, not the callback.

[mvolz]

Whoops, realise I suck at reading comprehension and it does say it in the directions.

[mvolz]

RE-opened because the code looks it could be providing a much nicer error:

See line 115 in make_it_modular/verify.js:

        if (noDotExp.length === list.length) {
          return modFileError(__('fail.mod.dotExt')) // Looks like this might be an appropriate error
        }

Instead, the error from the following block (line 121) is being thrown, which is the confusing message.

        //---- Check that we got the expected number of elements in the Array
        if (exp.length !== list.length) {
          return modFileError(__('fail.mod.array_wrong_size')) // TODO: Failing here instead
        }

[mvolz]

This is similar to #222 (but more specific, example of failing code with confusing message.)

[rjmunro]

It looks like this is partially fixed in #227. Other parts are covered by #298.