Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions implement-shell-tools/cat/cat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env node

const fs = require('fs');
const path = require('path');

function cat(files, options) {
let lineNumber = 1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable 'lineNumber' is declared outside the loop that goes through each file. This means if you process multiple files, the line numbers will continue from one file to the next, rather than restarting for each file. Is this the behavior you want? If not, where could you declare 'lineNumber' so that it resets for each file?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this - see #4 (comment)


files.forEach((file) => {
const filePath = path.resolve(file);

try {
const data = fs.readFileSync(filePath, 'utf8');
const lines = data.split('\n');

lines.forEach((line) => {
if (options.numberNonEmpty && line.trim()) {
console.log(`${lineNumber}\t${line}`);
lineNumber++;
} else if (options.numberLines) {
console.log(`${lineNumber}\t${line}`);
lineNumber++;
Comment on lines +17 to +22
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed that the logic for printing lines with or without numbers is repeated in two branches of your if-else statement (lines 17-22). If you ever wanted to change how lines are printed, you'd have to update it in both places. How could you extract this logic into a helper function to avoid repeating yourself and make future changes easier?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comment!

} else {
console.log(line);
}
});
} catch (err) {
console.error(`cat: ${file}: No such file or directory`);
}
});
}

function main() {
const args = process.argv.slice(2);
const options = {
numberLines: false,
numberNonEmpty: false,
Comment on lines +36 to +37
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The option names 'numberLines' and 'numberNonEmpty' are a bit similar and could be confusing. Could you think of names that make it clearer what each option does, especially for someone reading the code for the first time?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Borderline useful comment, though I'd maybe have phrased this as "so that if someone sees either one option name without seeing the other, they would know the difference"

};

const files = [];

args.forEach((arg) => {
if (arg === '-n') {
options.numberLines = true;
} else if (arg === '-b') {
options.numberNonEmpty = true;
} else {
files.push(arg);
}
});

if (files.length === 0) {
console.error('Usage: node cat.js [-n | -b] <file>...');
process.exit(1);
}

cat(files, options);
}

main();
44 changes: 44 additions & 0 deletions implement-shell-tools/ls/ls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/usr/bin/env node

const fs = require('fs');
const path = require('path');

function listFiles(directory, options) {
try {
const files = fs.readdirSync(directory, { withFileTypes: true });

files.forEach((file) => {
if (!options.all && file.name.startsWith('.')) {
return; // Skip hidden files unless -a is specified
}
console.log(file.name);
});
} catch (err) {
console.error(`ls: cannot access '${directory}': No such file or directory`);
}
}

function main() {
const args = process.argv.slice(2);
const options = {
all: false,
};

let directories = ['.'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You use a variable called 'directories', but in your code, it always contains only one directory (or the default '.'). Do you think the plural name might mislead someone into thinking it can handle multiple directories at once? What name might better reflect its actual use?


args.forEach((arg) => {
if (arg === '-1') {
// -1 is the default behavior, so no action needed
} else if (arg === '-a') {
options.all = true;
} else {
directories = [arg];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You use a variable called 'directories', but in your code, it always contains only one directory (or the default '.'). Do you think the plural name might mislead someone into thinking it can handle multiple directories at once? What name might better reflect its actual use?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this comment wouldn't be duplicated

}
});

directories.forEach((directory) => {
listFiles(directory, options);
});
}

main();
61 changes: 61 additions & 0 deletions implement-shell-tools/wc/wc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/usr/bin/env node

const fs = require('fs');
const path = require('path');

function countFile(filePath, options) {
try {
const data = fs.readFileSync(filePath, 'utf8');

const lines = data.split('\n').length;
const words = data.split(/\s+/).filter(Boolean).length;
const bytes = Buffer.byteLength(data, 'utf8');

if (options.lines) {
console.log(`${lines}\t${filePath}`);
} else if (options.words) {
console.log(`${words}\t${filePath}`);
} else if (options.bytes) {
console.log(`${bytes}\t${filePath}`);
} else {
console.log(`${lines}\t${words}\t${bytes}\t${filePath}`);
}
} catch (err) {
console.error(`wc: ${filePath}: No such file or directory`);
}
}

function main() {
const args = process.argv.slice(2);
const options = {
lines: false,
words: false,
bytes: false,
};

const files = [];

args.forEach((arg) => {
if (arg === '-l') {
options.lines = true;
} else if (arg === '-w') {
options.words = true;
} else if (arg === '-c') {
options.bytes = true;
} else {
files.push(arg);
}
});

if (files.length === 0) {
console.error('Usage: wc [-l | -w | -c] <file>...');
process.exit(1);
}

files.forEach((file) => {
const filePath = path.resolve(file);
countFile(filePath, options);
});
}

main();
Loading