London | 26-JUL-SDC | Ping Wang | Sprint 3 | Implement- shell- tools#585
London | 26-JUL-SDC | Ping Wang | Sprint 3 | Implement- shell- tools#585pathywang wants to merge 9 commits into
Conversation
pathywang
added
the
Needs Review
pathywang
added
Needs Review
SlideGauge
left a comment
SlideGauge
left a comment
There was a problem hiding this comment.
Good job, could you address a couple of notes from me?
| console.log(output.join(" ")); | ||
| } | ||
|
|
||
| for (const file of files) { |
There was a problem hiding this comment.
Run the real wc sample-files/* with several files — what extra line appears at the bottom that yours doesn't produce? Where in this loop could you accumulate the counts to print it?
| } | ||
|
|
||
| function countFile(filename) { | ||
| const content = fs.readFileSync(filename, "utf8"); |
There was a problem hiding this comment.
your ls example wraps the file access in try/catch block. Do we need the same block here?
|
|
||
| for (const file of files) { | ||
| const content = fs.readFileSync(file, "utf8"); | ||
| const lines = content.split("\n"); |
There was a problem hiding this comment.
If a file ends in \n, what is the last element of content.split("\n")? Combined with console.log adding its own newline, how does cat sample-files/1.txt compare to the real output — same lines, or one extra blank?
| let lineNumber = 1; | ||
|
|
||
| for (const file of files) { | ||
| const content = fs.readFileSync(file, "utf8"); |
There was a problem hiding this comment.
Do we need to guard this block in similar manner to ls?
There was a problem hiding this comment.
thanks, SlideGauge, i did complete correction as required. Hopefully you are happy with it
SlideGauge
added
Reviewed
pathywang
added
Needs Review
2 participants
Self checklist
Changelist
After i completed prep, created branch from main branch and felt the coursework is getting really hard because unable to get too much clue from prep so get lots of help and explanation from AL and review again in order to full understand knowledge