London | 25-SDC-July | Andrei Filippov | Sprint 3 | Implement shell tools by Droid-An · Pull Request #1 · CodeYourFuture/Module-Tools-Mirror-For-AI-Code-Reviewer-Testing

Droid-An · 2026-03-13T15:49:12Z

Learners, PR Template

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

Done shell tools

Questions

none

cyf-ai-code-reviewer · 2026-03-14T19:27:33Z

Thanks for opening a new PR! AI started to review it

cyf-ai-code-reviewer · 2026-03-14T19:27:45Z

implement-shell-tools/cat/cat.js

+async function example(path) {
+  try {
+    const data = await fs.readFile(path, { encoding: "utf8" });
+    if (opts["b"]) {
+      countLines(data);
+    } else if (opts["n"]) {
+      countLines(data);
+    } else {
+      console.log(data.trimEnd());
+    }
+  } catch (err) {
+    console.error(err);
+  }
+}


How do you usually choose names for your functions and variables? What could you rename 'example' and 'argv' to make their purposes clearer, both to you in the future and to someone else reading your code?

cyf-ai-code-reviewer · 2026-03-14T19:27:48Z

implement-shell-tools/cat/cat.js

+  for (const line of lines) {
+    if (opts.b) {
+      if (line.trim() === "") {
+        console.log();
+      } else {
+        console.log(`${lineNum} ${line}`);
+        lineNum++;
+      }
+    } else if (opts.n) {
+      console.log(`${lineNum} ${line}`);
+      lineNum++;
+    }
+  }


When you have two code sections that are very similar except for a small difference, is there a way you could combine them? What would that look like here?

cyf-ai-code-reviewer · 2026-03-14T19:27:51Z

implement-shell-tools/ls/ls.js

+  }
+})();
+
+const flag_1 = (files) => {


If you explained what each function and variable does to a friend who had never seen the code, what might you name them instead so they're easy to understand?

cyf-ai-code-reviewer · 2026-03-14T19:27:54Z

implement-shell-tools/ls/ls.js

+  if (programArgv.length === 1) {
+    pathToFile = programArgv[0];
+    try {
+      const stats = await fs.stat(pathToFile);
+      if (stats.isFile()) {
+        await listFiles("file");
+      } else if (stats.isDirectory()) {
+        listFiles("directory");
+      } else {
+        console.error("Not a file or directory.");
+      }
+    } catch (err) {
+      console.error("Invalid path:", err.message);
+    }
+  } else if (programArgv.length === 0) {
+    pathToFile = process.cwd();
+    await listFiles("directory");
+  } else {
+    console.error(
+      `Expected no more than 1 argument (sample-files) but got ${programArgv.length}.`
+    );
+  }
+})();


Is there a way you could structure your code so the decision of whether to list files, directories, or error messages, and how to display them, is handled in just one spot? Why might that make maintenance simpler over time?

cyf-ai-code-reviewer · 2026-03-14T19:27:57Z

implement-shell-tools/ls/ls.js

+
+program.parse();
+
+let pathToFile = "";


Can you spot a way to limit the scope of 'pathToFile' so it's only available where it's actually used? What changes would that require in your structure?

cyf-ai-code-reviewer · 2026-03-14T19:28:00Z

implement-shell-tools/wc/wc.js

+  if (opts["l"]) {
+    outputPerFile.push(flag_l(content));
+  }
+  if (opts["w"]) {
+    outputPerFile.push(flag_w(content));
+  }
+  if (opts["c"]) {
+    outputPerFile.push(flag_c(content));
+  }


If you look for patterns in the way you're handling each of the '-l', '-w', and '-c' options, can you imagine a way to combine the operations to make your code shorter and easier to change in the future?

cyf-ai-code-reviewer · 2026-03-14T19:28:03Z

implement-shell-tools/wc/wc.js

+const total = [];
+const output = [];
+const countsPerFile = [];
+let columnWidth = 0;


What would happen if you moved the declaration of 'columnWidth' inside 'handleInput' instead of declaring it outside? How might that affect readability and maintainability for you or someone else looking at your code?

cyf-ai-code-reviewer · 2026-03-14T19:28:06Z

implement-shell-tools/cat/cat.js

+  .argument("<sample-files...>", "The file path to process")
+  .parse();
+
+const argv = program.args;


How do you usually choose names for your functions and variables? What could you rename 'example' and 'argv' to make their purposes clearer, both to you in the future and to someone else reading your code?

cyf-ai-code-reviewer · 2026-03-14T19:28:09Z

implement-shell-tools/ls/ls.js

+  });
+};
+
+const flag_a = (files) => {


If you explained what each function and variable does to a friend who had never seen the code, what might you name them instead so they're easy to understand?

cyf-ai-code-reviewer · 2026-03-14T19:28:12Z

implement-shell-tools/ls/ls.js

+async function listFiles(type) {
+  let output = [];
+  let formattedPath = "";
+  if (type == "directory") {
+    formattedPath = pathToFile;
+  } else if (type == "file") {
+    formattedPath = path.dirname(pathToFile);
+  }
+  const char = program.opts();
+  const files = await fs.readdir(formattedPath);
+  const sortedOutput = files.sort((a, b) => a.localeCompare(b));
+
+  if (char["a"]) {
+    output = flag_a(sortedOutput);
+  } else {
+    sortedOutput.forEach(function (file) {
+      if (file[0] != ".") {
+        output.push(file);
+      }
+    });
+  }
+
+  if (char["1"]) {
+    flag_1(output);
+  } else {
+    console.log(output.join("   "));
+  }
+}


Is there a way you could structure your code so the decision of whether to list files, directories, or error messages, and how to display them, is handled in just one spot? Why might that make maintenance simpler over time?

cyf-ai-code-reviewer · 2026-03-14T19:28:15Z

implement-shell-tools/ls/ls.js

+
+(async () => {
+  if (programArgv.length === 1) {
+    pathToFile = programArgv[0];


Can you spot a way to limit the scope of 'pathToFile' so it's only available where it's actually used? What changes would that require in your structure?

cyf-ai-code-reviewer · 2026-03-14T19:28:18Z

implement-shell-tools/wc/wc.js

+  for (const row of output) {
+    for (let i = 0; i < numOfColumns; i++) {
+      columnWidth = Math.max(columnWidth, String(row[i]).length);
+    }
+  }
+
+  for (let row of output) {
+    const line_parts = [];
+    for (let i = 0; i < numOfColumns; i++) {
+      line_parts.push(String(row[i]).padStart(columnWidth + 4));
+    }


What would happen if you moved the declaration of 'columnWidth' inside 'handleInput' instead of declaring it outside? How might that affect readability and maintainability for you or someone else looking at your code?

cyf-ai-code-reviewer · 2026-03-14T19:28:21Z

implement-shell-tools/ls/ls.js

+  } else if (type == "file") {
+    formattedPath = path.dirname(pathToFile);
+  }
+  const char = program.opts();


If you explained what each function and variable does to a friend who had never seen the code, what might you name them instead so they're easy to understand?

cyf-ai-code-reviewer · 2026-03-14T19:28:24Z

implement-shell-tools/ls/ls.js

+      console.error("Invalid path:", err.message);
+    }
+  } else if (programArgv.length === 0) {
+    pathToFile = process.cwd();


Can you spot a way to limit the scope of 'pathToFile' so it's only available where it's actually used? What changes would that require in your structure?

cyf-ai-code-reviewer · 2026-03-14T19:28:27Z

implement-shell-tools/ls/ls.js

+    formattedPath = pathToFile;
+  } else if (type == "file") {
+    formattedPath = path.dirname(pathToFile);


Can you spot a way to limit the scope of 'pathToFile' so it's only available where it's actually used? What changes would that require in your structure?

cyf-ai-code-reviewer · 2026-03-15T16:36:44Z

Thanks for opening a new PR! AI started to review it

cyf-ai-code-reviewer · 2026-03-15T16:36:58Z

implement-shell-tools/cat/cat.js

+async function example(path) {
+  try {
+    const data = await fs.readFile(path, { encoding: "utf8" });
+    if (opts["b"]) {
+      countLines(data);
+    } else if (opts["n"]) {
+      countLines(data);
+    } else {
+      console.log(data.trimEnd());
+    }
+  } catch (err) {
+    console.error(err);
+  }
+}


How do you decide on meaningful names for your functions? Can you think of a name that might better communicate this function's purpose?

cyf-ai-code-reviewer · 2026-03-15T16:36:58Z

implement-shell-tools/cat/cat.js

+    const data = await fs.readFile(path, { encoding: "utf8" });
+    if (opts["b"]) {
+      countLines(data);
+    } else if (opts["n"]) {
+      countLines(data);
+    } else {


When two different options lead to almost the same behavior, what pattern could you use to reduce duplicated code and keep it easier to update later?

cyf-ai-code-reviewer · 2026-03-15T16:36:58Z

implement-shell-tools/cat/cat.js

+  for (const line of lines) {
+    if (opts.b) {
+      if (line.trim() === "") {
+        console.log();
+      } else {
+        console.log(`${lineNum} ${line}`);
+        lineNum++;
+      }
+    } else if (opts.n) {
+      console.log(`${lineNum} ${line}`);
+      lineNum++;
+    }
+  }


When two different options lead to almost the same behavior, what pattern could you use to reduce duplicated code and keep it easier to update later?

cyf-ai-code-reviewer · 2026-03-15T16:36:58Z

implement-shell-tools/ls/ls.js

+
+program.parse();
+
+let pathToFile = "";


Why do you think keeping variables inside the smallest possible scope is a helpful practice? Can you imagine how this variable could be kept within the async block or passed as a parameter instead of being an outer variable?

cyf-ai-code-reviewer · 2026-03-15T16:36:58Z

implement-shell-tools/ls/ls.js

+
+(async () => {
+  if (programArgv.length === 1) {
+    pathToFile = programArgv[0];


Why do you think keeping variables inside the smallest possible scope is a helpful practice? Can you imagine how this variable could be kept within the async block or passed as a parameter instead of being an outer variable?

cyf-ai-code-reviewer · 2026-03-15T16:36:59Z

implement-shell-tools/ls/ls.js

+  if (type == "directory") {
+    formattedPath = pathToFile;
+  } else if (type == "file") {
+    formattedPath = path.dirname(pathToFile);


Why do you think keeping variables inside the smallest possible scope is a helpful practice? Can you imagine how this variable could be kept within the async block or passed as a parameter instead of being an outer variable?

cyf-ai-code-reviewer · 2026-03-15T16:36:59Z

implement-shell-tools/ls/ls.js

+const flag_1 = (files) => {
+  files.forEach(function (file) {
+    console.log(file);
+  });
+};


When you name your functions, how do you decide on a name? What might be some more descriptive names for flag_1?

cyf-ai-code-reviewer · 2026-03-15T16:36:59Z

implement-shell-tools/ls/ls.js

+    sortedOutput.forEach(function (file) {
+      if (file[0] != ".") {
+        output.push(file);
+      }
+    });


Have you encountered array filter methods before? How could you use them to make this block of code shorter or clearer?

cyf-ai-code-reviewer · 2026-03-15T16:36:59Z

implement-shell-tools/wc/wc.js

+const countAndDisplay = async (path) => {
+  const outputPerFile = [];
+  const content = await fs.readFile(path, "utf-8");
+  if (opts["l"]) {
+    outputPerFile.push(flag_l(content));
+  }
+  if (opts["w"]) {
+    outputPerFile.push(flag_w(content));
+  }
+  if (opts["c"]) {
+    outputPerFile.push(flag_c(content));
+  }
+  if (argv.length > 1) {
+    if (total.length == 0) {
+      total.push(...outputPerFile);
+    } else {
+      for (let index = 0; index < outputPerFile.length; index++) {
+        total[index] += outputPerFile[index];
+      }
+    }
+    countsPerFile.push(...outputPerFile);
+  }
+  outputPerFile.push(path);
+  output.push([...outputPerFile]);
+};


If you wanted to adapt this code to work differently (for example, outputting results in a different format), how might breaking this apart into smaller functions make it easier to change?

cyf-ai-code-reviewer · 2026-03-15T16:36:59Z

implement-shell-tools/wc/wc.js

+  if (opts["l"]) {
+    outputPerFile.push(flag_l(content));
+  }
+  if (opts["w"]) {
+    outputPerFile.push(flag_w(content));
+  }
+  if (opts["c"]) {
+    outputPerFile.push(flag_c(content));
+  }


What would happen if you had to add more options like -m for characters? Can you imagine a way to pair up the options with their processing functions so the code to handle them doesn't need to be written out every time?

cyf-ai-code-reviewer · 2026-03-15T19:24:42Z

Thanks for opening a new PR! AI started to review it

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/ls/ls.js

+async function listFiles(type) {
+  let output = [];
+  let formattedPath = "";
+  if (type == "directory") {
+    formattedPath = pathToFile;
+  } else if (type == "file") {
+    formattedPath = path.dirname(pathToFile);
+  }
+  const char = program.opts();
+  const files = await fs.readdir(formattedPath);
+  const sortedOutput = files.sort((a, b) => a.localeCompare(b));
+
+  if (char["a"]) {
+    output = flag_a(sortedOutput);
+  } else {
+    sortedOutput.forEach(function (file) {
+      if (file[0] != ".") {
+        output.push(file);
+      }
+    });
+  }
+
+  if (char["1"]) {
+    flag_1(output);
+  } else {
+    console.log(output.join("   "));
+  }
+}


What do you think a better name for the function that handles both files and directories could be? Does calling it 'listFiles' match what you expect it to do when reading code?

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/ls/ls.js

+const flag_a = (files) => {
+  files.unshift("..");
+  files.unshift(".");
+  return files;
+};


How could you ensure that you're not duplicating '.' and '..' entries if flag_a gets called several times on the same array? What do you think would happen currently if someone tried that?

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/ls/ls.js

+
+program.parse();
+
+let pathToFile = "";


What would be the effect of declaring pathToFile inside your main async function, instead of at the top? Would this make it clearer where it's used and how it's set?

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/wc/wc.js

+const total = [];
+const output = [];
+const countsPerFile = [];


Do you see a way to combine or clarify these arrays so it's obvious what each one does, or maybe structure your results a little differently for clarity and future flexibility?

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/wc/wc.js

+const flag_l = (content) => {
+  return content.split("\n").length - 1;
+};


How could you ensure that the count matches typical behavior, especially for files that may or may not end with a newline? Could you test with different kinds of files and see what outputs you get?

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/cat/cat.js

+    if (opts["b"]) {
+      countLines(data);
+    } else if (opts["n"]) {
+      countLines(data);
+    } else {


Is there a case where both -b and -n could be set? What does the standard tool do if this happens? Could you adjust your logic to handle this in a way that would make sense to users?

cyf-ai-code-reviewer · 2026-03-15T19:24:55Z

implement-shell-tools/cat/cat.js

+async function example(path) {
+  try {
+    const data = await fs.readFile(path, { encoding: "utf8" });
+    if (opts["b"]) {
+      countLines(data);
+    } else if (opts["n"]) {
+      countLines(data);
+    } else {
+      console.log(data.trimEnd());
+    }
+  } catch (err) {
+    console.error(err);
+  }
+}


Is there another function name that would help a future reader quickly understand what this function does? What about something like 'processFile'?

Droid-An added 3 commits March 13, 2026 15:45

Implement cat command to concatenate files

8cd986e

Implement ls command to list directory contents

cf41592

Create wc.js

17c32ec

This comment has been minimized.

Sign in to view

Droid-An added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 13, 2026

This comment has been minimized.

Sign in to view

github-actions bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 13, 2026

cyf-ai-code-reviewer bot reviewed Mar 14, 2026

View reviewed changes

Droid-An added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 15, 2026

cyf-ai-code-reviewer bot reviewed Mar 15, 2026

View reviewed changes

Droid-An added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 15, 2026

cyf-ai-code-reviewer bot reviewed Mar 15, 2026

View reviewed changes

Conversation

Droid-An commented Mar 13, 2026

Learners, PR Template

Changelist

Questions

Uh oh!

This comment has been minimized.

This comment has been minimized.

cyf-ai-code-reviewer bot commented Mar 14, 2026

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot commented Mar 15, 2026

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

cyf-ai-code-reviewer bot Mar 15, 2026

Choose a reason for hiding this comment