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;

files.forEach((file) => {
const filePath = path.resolve(file);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this line doing? What would break if you removed it and just used file instead of filePath?


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++;
} else {
console.log(line);
Comment on lines +17 to +24
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The three branches here look quite similar and repetitive. In general, if you have multiple similar branches, it's more clear to extract the differences into variables, and then run the same code, i.e. so you'd only have one call to console.log which looks more like console.log(`${prefix}${line}\n`) where prefix may be set differently based on options (including potentially an empty string).

This way it's easier for someone reading the code to see what's the same / different in each case, and also avoids the hazard that someone updates one of the branches but forgets to update the other ones.

}
});
} catch (err) {
console.error(`cat: ${file}: No such file or directory`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What exit code will your program have if something went wrong? What exit code should it have?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you sure the problem here is that the file didn't exist? What would happen e.g. if you didn't have permission to read the file?

In general specific error messages are good, but misleadingly specific error messages are a problem - if we're not sure what went wrong (or if we have more information about what went wrong), we should present that information, rather than guessing. And if we are guessing, we should make it clear we're not sure what the problem exactly was and that this is a guess.

}
});
}

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

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 = ['.'];

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
Member

Choose a reason for hiding this comment

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

This meets the requirements for the examples listed in the README.md, but feels like it risks being confusing to users. If someone specified ls /some/path /some/other/path what do you think they would expect to happen? What does your code actually do? (See also the comment about wc options for a similar question)

}
});

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);
}
Comment on lines +39 to +47
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens if someone specifies more than one of these flags? What should happen?

Given the test cases we gave you in the README file, it's ok if your implementation doesn't do the same thing as the real wc does, though that would be ideal, but in general ignoring user input is bad - so if someone asks for both -l and -c and you ignore one of them, that can be confusing. Either showing both, or giving an error, is probably preferable.

});

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

files.forEach((file) => {
const filePath = path.resolve(file);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, why do you need the path.resolve here?

countFile(filePath, options);
});
}

main();
Loading