Uh oh!
There was an error while loading. Please reload this page.
London | 26-SDC-July | Alex Jamshidi | Sprint 3 | Implement shell tools - #635
London | 26-SDC-July | Alex Jamshidi | Sprint 3 | Implement shell tools #635Alex-Jamshidi wants to merge 5 commits into
Conversation
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start but there is quite a bit of work still to do.
You've built your own argument handling system. Is your way of handling options/arguments always going to be the best? Is there a better way of doing this?
| function bFlag() { | ||
| return allFilesContents.map((fileContent) => { | ||
| let lineNumber = 1; | ||
| return fileContent.map((line, index) => { |
There was a problem hiding this comment.
A map is typically used to change one array into another using a function that doesn't have side effects. Here, you are changing a lineNumber variable each time the map runs. This isn't wrong, but just be careful of side effects, especially if you ever want to run things asynchronously.
| function Flag1() { | ||
| if (printInList) { | ||
| outputString = outputString.replaceAll("\t", "\n"); |
There was a problem hiding this comment.
I like that you are using lots of task specific functions.
But, is using global variables in this way a safe way of programming this? Is there a way that would have the data flow through your app in a more semantic way?
| } | ||
| function print() { | ||
| if (printInList == true) { |
There was a problem hiding this comment.
Think about how if conditions work, and check this line again. Are you writing code in the most optimal way?
| } | ||
| function deleteOutputs() { | ||
| if (!deleted) { |
There was a problem hiding this comment.
Can you explain your thinking that led to a design where you need to manually handle your resources like this?
Learners, PR Template
Self checklist
Changelist
cat
Handles all commands correctly:
cat sample-files/1.txt
cat -n sample-files/1.txt
cat sample-files/.txt
cat -n sample-files/.txt
cat -b sample-files/3.txt
Stretch:
Handles multiple flags and incorrect flags
(although does not parse b and n correctly when used together)
wc
Handles all commands correctly:
wc sample-files/*
wc -l sample-files/3.txt
wc -w sample-files/3.txt
wc -c sample-files/3.txt
wc -l sample-files/*
wc -w -l sample-files/3.txt
wc -w -l sample-files/*
Stretch:
Handles multiple flags and incorrect flags
Handles flag order
ls
Handles all commands correctly:
ls sample-files
ls sample-files/1.txt
ls sample-files/
ls -1
ls -1 sample-files
ls -1 -a sample-files
Stretch
Handles multiple flags and incorrect flags
Handles flag order
Task ID: CYF-1150