Skip to content

not quite working yet#12

Open
0blu3 wants to merge 1 commit intocodefellows-javascript-401d13:masterfrom
0blu3:master
Open

not quite working yet#12
0blu3 wants to merge 1 commit intocodefellows-javascript-401d13:masterfrom
0blu3:master

Conversation

@0blu3
Copy link
Copy Markdown

@0blu3 0blu3 commented Feb 16, 2017

No description provided.

"indent": [ "error", 2 ],
"quotes": [ "error", "single" ],
"semi": ["error", "always"],
"linebreak-style": [ "error", "windows" ]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boo. No comma dangle. :p

@@ -0,0 +1,117 @@
# Created by https://www.gitignore.io/api/node,vim,osx,macos,linux

*node_modules
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work ignoring node modules.


const fs = require('fs');

const textReader = module.exports = function(file, callback) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good function here. This is the right structure, but it doesn't ensure that this will run asynchronously.


const textReader = require('../lib/text-reader.js');

textReader('./data/one.txt', function(err, data) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good function structure here.

@@ -0,0 +1,23 @@
'use strict';

var buffs = [];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work storing your results in your array...but where is it going? I see you pushing all of the data from your function calls into this array, but you're not passing it back to the function you're calling.


const textReader = module.exports = function(file, callback) {
fs.readFile(file, function(err, data) {
if (err) return callback(err);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work returning the callback with an error.

const expect = require('chai').expect;
const textReader = require('../lib/text-reader.js');
const buffs = require('buffs');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your tests test that each of your files is being read, but it is not testing if they are being read asynchronously, and if the information is coming back in the right order.

});
});
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need a test that can read all three text files in the same function, do it IN ORDER asynchronously, and return the information in the proper order. You should test for that order being correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants