London | 26-SDC-July | Alex Jamshidi | Sprint 4 | Implement shell tools in python

[Alex-Jamshidi]

Learners, PR Template

Self checklist

• I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
• My changes meet the requirements of the task
• I have tested my changes
• My changes follow the style guide

Task code

CYF-1152

Changelist

Implemented wc, ls, cat in python

[LonMcGregor]

Good start on this task. Reading through it, I can see you have put thought into these implementations, but they feel a little bit over-engineered, and at times are a bit difficult to see how the data is flowing through them. I've got a few questions for you to answer which might help me better understand your thinking.

[LonMcGregor]

Is there a reason to use two loops here? Could your data structure be simplified?

[Alex-Jamshidi]

See above comment.

[LonMcGregor]

Here it looks like you loop over all the content twice.Once to add the line numbers, then once again to print. Is it possible to do this only looking at the data once?

[Alex-Jamshidi]

There is a reason for this. The data in all_file_contents is stored as two lots of nested arrays.

For 3 files it might look like this:
[[line, line, line, line], [line, line, line], [line, line, line, line]]

For a single file it looks like this:
[[line, line, line]]

So the first loop essentially just unwraps that first array.

I could update the code in read_files() to unpack all lines into a single array, even if there are multiple files... that would be something like...

    file_content = read_file(file_name, cwd).splitlines())
    for line in file_content:
    all_files_contents.append(line)

(replacing lines 29 and 30).

This would then let me have a single loop in each of the two locations you flagged:
for line in all_file_contents:

But I chose to do it this way to preserve the information of which lines are from which files. The reason to do this was because, this version of cat implements only 2 flags, but was written with the view to be potentially expanded, so I wanted to maintain as much information about the input as possible, letting the print function unpack it all at the end.

This is also why there are two loops below.

[LonMcGregor]

Here, when you are doing things like if args.a …, then you set something to True. What benefit is there doing it this way over just referencing args.a when you need it?

[LonMcGregor]

What do you use the cwd for in this program?

[LonMcGregor]

There are multiple places where you use remove_dot_files in this program. Is there a reason for that?

[LonMcGregor]

Why are you using the Path / expression here?

[LonMcGregor]

Is there a reason the word count method has a different structure to your line count method?

[LonMcGregor]

Do you need to loop over metrics and then check if it is in displayed_metrics? Could you simplify this?