-
Notifications
You must be signed in to change notification settings - Fork 15
lab-dana #13
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
base: master
Are you sure you want to change the base?
lab-dana #13
Conversation
…gitignore created first.
…method but realized that I should do a commit first.
…lp hoping to add automation script.
@@ -0,0 +1,3 @@ | |||
Enamel pin fashion axe cred, offal tilde paleo glossier art party unicorn narwhal chicharrones church-key hexagon small batch. Photo booth helvetica quinoa, thundercats lumbersexual deep v mixtape pour-over single-origin coffee whatever. Fam scenester edison bulb farm-to-table, cornhole kinfolk seitan pabst 90's kombucha la croix normcore activated charcoal mumblecore. Polaroid before they sold out 8-bit, etsy pitchfork man bun normcore succulents knausgaard humblebrag franzen kitsch marfa organic actually. Franzen knausgaard kinfolk etsy ugh. Bitters roof party tumblr hammock. Gochujang kinfolk pickled enamel pin jean shorts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay lorem ipsum stuff. :)
index.js
Outdated
// | ||
// const reader = require('./lib/file-reader.js').readFiles; | ||
// | ||
// reader(['one.txt', 'two.txt', 'three.txt']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll want to be able to call your method here in your index with all 3 of your files, and your callback function as your final argument to the function.
// | ||
// const fs = require('fs'); | ||
// | ||
// const dataFiles = module.exports = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is heading in the correct direction. You need to figure out how to structure your function dataFils so that it can take the files you want to read, and then take a callback function as a final argument.
lib/file-reader.js
Outdated
if (file instanceof Array) {//linter wouldn't let me use (! instanceof) | ||
file.map(function(ele) { | ||
fs.readFile(`${__dirname}/../data/${ele}`, function(err, data) { | ||
if (err) return callback(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, this is the correct pattern for handling an error if it comes up and returning it.
describe('File Reader Module', function() { | ||
describe('test false file path', function() { | ||
it('should return an error message', function(done) { | ||
readFile(['imaginary-file.txt'], function(err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good test; this should work fine when your method is working properly.
test/file-reader-test.js
Outdated
it('should return the first 8 hex digits', function(done) { | ||
readFile(['one.txt', 'two.txt', 'three.txt'], function(err, data) { | ||
expect(err).to.equal(null); | ||
expect(data).to.be.a('string'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, but for this test you'll want to ensure that the three files come back in the proper order. Good way to do that would be to check the values of the returning data's indices at [0], then [1], then [2].
Test is structured mostly right, though.
I think that I misunderstood the assignment and maybe tried to overcomplicated it.