Skip to content

not quite working yet #12

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0blu3
Copy link

@0blu3 0blu3 commented Feb 16, 2017

No description provided.

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

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

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) {

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) {

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 = [];

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);

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');

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.

});
});
});
});

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