Conversation
Fix expected output: change 'with' to 'wIth' in sed exercise instructions
|
Thanks for opening a new PR! AI started to review it |
| // filter flags, and getting the string of filename | ||
| const filePaths = argv.filter((arg) => !arg.startsWith("-")); | ||
|
|
||
| const noFlags = !showLines && !showCharacters && !showWords && !showCharacters; |
There was a problem hiding this comment.
The variable 'noFlags' is intended to check if no flags are set, but the logic includes '!showCharacters' twice and omits '!showBytes'. This could be confusing to someone reading the code. How could you make the naming and logic clearer so that it accurately reflects the intent and is easy to understand?
| let output = ""; | ||
|
|
||
| if (showLines || noFlags) output += `${lines} `; | ||
| if (showWords || noFlags) output += `${words} `; | ||
| if (showBytes || noFlags) output += `${bytes} `; | ||
| if (showCharacters || noFlags) output += `${characters} `; |
There was a problem hiding this comment.
You assign the result of fs.readFileSync(filePath, "utf-8") to 'content', and then use it immediately for processing. Since you use 'content' multiple times, this seems reasonable. However, for the 'output' variable, you build up a string by appending to it in several if statements. Do you think there is a clearer way to construct the output string, perhaps by collecting the values in an array and joining them, to avoid the need for a mutable variable?
There was a problem hiding this comment.
@Droid-An This comment seems quite odd?
The "content"-related stuff feels irrelevant. And I'm not sure accumulating in an array is any cleaner than mutating a string? There may be other benefits (e.g. avoiding O(n^2) copies repeatedly appending), but avoiding a mutable variable by using something that happens to be internally mutable because it's a reference type seems like an odd suggestion?
No description provided.