Glasgow | 25-SDC-NOV | Katarzyna Kazimierczuk | Sprint 3 | Implement Shell Tools

[katarzynakaz]

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

Changelist

js tools.

[SlideGauge]

General note: I notced async/await being used here, I have doubts in necessity of it in this context: we're doing a console utility

[katarzynakaz]

Thank you for the feedback. I was folllowing the coursework in Sp 4 with the use of async / await but I see your point.
Changes implemented :)

[illicitonion]

A lot of the pieces here are looking good, but the solutions often aren't very general - can you take a look at these comments? Thanks!

[illicitonion]

We generally don't want to hard-code that our program only works with specific directories - can you think how to make this more general? And how to make it share code with the other two functions?

[illicitonion]

The real wc also supports passing multiple of the flags at once (e.g. wc -w -l /some/file), and does something a bit different if you pass multiple files (wc -w -c /some/file /some/other/file) - can you see if you can match those behaviours?

[illicitonion]

These functions are quite repetitive with each other.

[illicitonion]

Please remove the assorted commented out code :)

[illicitonion]

These ifs are quite repetitive, which has a couple of issues:

1. When reading the code, it's hard to see which parts are the same across the branches, and what parts are different.
2. In the future if we add new concerns (e.g. a "colour every other line" flag), we'll need to implement that across each of the branches.

We often solve this by separating "the differences" from "what's the same", e.g.

// Make a variable up-front:
let prefix = "";
// Conditionally change things
if (something) {
  prefix = "hello";
} else if (somethingElse) {
  prefix = "goodbye";
}
console.log(`${prefix}${line}`);

So that the ifs at the start make clear how things are different, and what comes after shows what's the same/different

[github-actions]

The changed files in this PR don't match what is expected for this task.

Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints.

Please review the changed files tab at the top of the page, we are only expecting changes in this directory: ^implement-shell-tools/

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

[github-actions]

The changed files in this PR don't match what is expected for this task.

Please check that you committed the right files for the task, and that there are no accidentally committed files from other sprints.

Please review the changed files tab at the top of the page, we are only expecting changes in this directory: ^implement-shell-tools/

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

[illicitonion]

Please can you remove these .DS_Store files from your branch? Thanks!