London | 26-Jul-SDC | Roman Sanaye | Sprint 4 | Implement shell tool in Python - #637
London | 26-Jul-SDC | Roman Sanaye | Sprint 4 | Implement shell tool in Python#637RomanSanaye wants to merge 6 commits into
Conversation
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start, but some things to fix.
also, Is the choice you have used for processing arguments the most effective? Is there a library you could use to help the implementation?
| # Print total only when multiple files | ||
| if len(paths) > 1: | ||
|
|
||
| total_output = [] |
There was a problem hiding this comment.
What do you think about the output code here? Can you spot any duplication across the file?
There was a problem hiding this comment.
Thanks. I removed the duplicated output logic by creating a get_output() function.
|
|
||
| else: | ||
| if "-l" in flags: | ||
| output.append(lines) |
There was a problem hiding this comment.
If you run this with multiple files, how does the output look? What change could make it neater?
There was a problem hiding this comment.
Thanks. I’ve updated the output formatting to make the results neater when using multiple files.
Hi sir @LonMcGregor |
LonMcGregor
left a comment
There was a problem hiding this comment.
These argparse implementations are better, and good idea in wc to write a function to handle printing output to reduce duplication.
I have some concern that you are not using the argparse library to best effect.
In particular, if we look at the cat implementation, you use this:
if args.n:
flag = "-n"
followed later by
if flag == "-n":
Is storing things as strings even necessary when you are using a library to help you in this way?
Similar issues exist across the other implementations.
Learners, PR Template
Self checklist
Changelist
Implemented the
wcshell tool using Python.The implementation supports:
-l,-w, and-cflagsI tested the program against the required commands:
wc sample-files/*wc -l sample-files/3.txtwc -w sample-files/3.txtwc -c sample-files/3.txtwc -l sample-files/*wc -w -l sample-files/3.txtwc -w -l sample-files/*Questions
No questions.
Task ID: CYF-1152