-
-
Notifications
You must be signed in to change notification settings - Fork 107
London | 26-SDC-July | Alex Jamshidi | Sprint 3 | Implement shell tools #635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
78bdfd5
60b323e
3a35a7a
085224e
b28f41d
7d6a298
fce72f0
14d4d33
2b981ed
fa7436b
d4720b1
5c64429
769706f
65e60ae
efdd20b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| const fs = require("fs"); | ||
|
|
||
| const arguments = process.argv; | ||
| const userArguments = arguments.slice(2); | ||
|
|
||
| const index = arguments[1].lastIndexOf("/"); | ||
| const currentWorkingDirectory = arguments[1].slice(0, index + 1); | ||
|
|
||
| const flags = userArguments | ||
| .filter((argument) => argument.startsWith("-")) | ||
| .map((argument) => argument.slice(1)) | ||
| .join(""); | ||
|
|
||
| const flagHandlers = { | ||
| b: bFlag, | ||
| n: nFlag, | ||
| }; | ||
|
|
||
| const fileNames = userArguments.filter((argument) => !argument.startsWith("-")); | ||
| let allFilesContents = []; | ||
|
|
||
| readFiles(); | ||
| executeFlags(); | ||
| printLines(); | ||
|
|
||
| function readFiles() { | ||
| fileNames.forEach((fileName) => { | ||
| fileContent = readFile(fileName); | ||
| allFilesContents.push(fileContent.split("\n")); | ||
| }); | ||
| } | ||
|
|
||
| function readFile(fileName) { | ||
| const filePath = currentWorkingDirectory + fileName; | ||
| return fs.readFileSync(filePath, "utf8").trimEnd(); | ||
| } | ||
|
|
||
| function executeFlags() { | ||
| for (const flag of flags) { | ||
| if (flagHandlers[flag]) { | ||
| allFilesContents = flagHandlers[flag](); | ||
| } else { | ||
| console.error(`cat: illegal option -- ${flag}\nusage: cat [-belnstuv] [file ...]`); | ||
| process.exit(1); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function bFlag() { | ||
| return allFilesContents.map((fileContent) => { | ||
| let lineNumber = 1; | ||
| return fileContent.map((line, index) => { | ||
| if (line == "") { | ||
| return `${line}`; | ||
| } | ||
| return `${String(lineNumber++).padStart(6, " ")} ${line}`; | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function nFlag() { | ||
| return allFilesContents.map((fileContent) => { | ||
| return fileContent.map((line, index) => `${String(index + 1).padStart(6, " ")} ${line}`); | ||
| }); | ||
| } | ||
|
|
||
| function printLines() { | ||
| allFilesContents.forEach((fileContent) => { | ||
| fileContent.forEach((line) => { | ||
| console.log(line); | ||
| }); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| const fs = require("fs"); | ||
| const { allowedNodeEnvironmentFlags } = require("process"); | ||
|
|
||
| const args = process.argv; | ||
| const userArgs = args.slice(2); | ||
|
|
||
| const index = args[1].lastIndexOf("/"); | ||
| const currentWorkingDirectory = args[1].slice(0, index); | ||
|
|
||
| const flags = userArgs | ||
| .filter((arg) => arg.startsWith("-")) | ||
| .map((arg) => arg.slice(1)) | ||
| .join(""); | ||
|
|
||
| const flagHandlers = { | ||
| 1: Flag1, | ||
| a: Flaga, | ||
| }; | ||
|
|
||
| let printInList = false; | ||
| let showAll = false; | ||
|
|
||
| const fsItems = userArgs.filter((arg) => !arg.startsWith("-")); | ||
| let dirArgs; | ||
| let fileArgs; | ||
| let outputString = ""; | ||
|
|
||
| checkArgsLength(); | ||
| executeFlags(); | ||
| filterFilesAndDirs(); | ||
| populateOutput(); | ||
| print(); | ||
|
|
||
| function executeFlags() { | ||
| for (const flag of flags) { | ||
| if (flagHandlers[flag]) { | ||
| allFilesContents = flagHandlers[flag](); | ||
| } else { | ||
| console.error( | ||
| `ls: invalid option -- ${flag}\nusage: ls [-@ABCFGHILOPRSTUWXabcdefghiklmnopqrstuvwxy1%,] [--color=when] [-D format] [file ...]`, | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function Flag1() { | ||
| if (printInList) { | ||
| outputString = outputString.replaceAll("\t", "\n"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that you are using lots of task specific functions. But, is using global variables in this way a safe way of programming this? Is there a way that would have the data flow through your app in a more semantic way?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have rewritten many of the functions to allow the data to flow through, rather than the functions independently manipulating data stored in global variables. |
||
| outputString = outputString.replaceAll("\n\n", "\n"); | ||
| } else { | ||
| printInList = true; | ||
| } | ||
| } | ||
|
|
||
| function Flaga() { | ||
| showAll = true; | ||
| } | ||
|
|
||
| function checkArgsLength() { | ||
| if (fsItems.length == 0) { | ||
| fsItems.push("."); | ||
| } | ||
| } | ||
|
|
||
| function filterFilesAndDirs() { | ||
| dirArgs = fsItems.filter((p) => fs.statSync(currentWorkingDirectory + "/" + p).isDirectory()); | ||
| fileArgs = fsItems.filter((p) => !fs.statSync(currentWorkingDirectory + "/" + p).isDirectory()); | ||
| if (showAll == false) { | ||
| removeDotFiles(fileArgs); | ||
| } | ||
| } | ||
|
|
||
| function populateOutput() { | ||
| if (fsItems.length == 1) { | ||
| fileArgs.forEach((file) => { | ||
| outputString += file + " "; | ||
| }); | ||
| dirArgs.forEach((dir) => { | ||
| outputString += dirOutput(dir); | ||
| }); | ||
| } else { | ||
| fileArgs.forEach((file) => { | ||
| outputString += file + "\t"; | ||
| }); | ||
| dirArgs.forEach((dir) => { | ||
| outputString += "\n\n" + dir + ":\n" + dirOutput(dir); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| function dirOutput(dir) { | ||
| let contents = fs.readdirSync(currentWorkingDirectory + "/" + dir); | ||
| let outputStr = ""; | ||
| if (showAll) { | ||
| outputStr += ".\t..\t"; | ||
| } else { | ||
| for (let i = contents.length - 1; i >= 0; i--) { | ||
| contents = removeDotFiles(contents); | ||
| } | ||
| } | ||
| contents.forEach((item) => { | ||
| outputStr += item + "\t"; | ||
| }); | ||
| return outputStr; | ||
| } | ||
|
|
||
| function removeDotFiles(fileList) { | ||
| for (let i = fileList.length - 1; i >= 0; i--) { | ||
| if (fileList[i][0] == ".") { | ||
| fileList.splice(i, 1); | ||
| } | ||
| } | ||
| return fileList; | ||
| } | ||
|
|
||
| function print() { | ||
| if (printInList == true) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think about how if conditions work, and check this line again. Are you writing code in the most optimal way?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah yes, I see - updated |
||
| Flag1(); | ||
| } | ||
| console.log(outputString.trimEnd()); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| const fs = require("fs"); | ||
| const { allowedNodeEnvironmentFlags } = require("process"); | ||
|
|
||
| const arguments = process.argv; | ||
| const userArguments = arguments.slice(2); | ||
|
|
||
| const index = arguments[1].lastIndexOf("/"); | ||
| const currentWorkingDirectory = arguments[1].slice(0, index + 1); | ||
|
|
||
| const flags = userArguments | ||
| .filter((argument) => argument.startsWith("-")) | ||
| .map((argument) => argument.slice(1)) | ||
| .join(""); | ||
|
|
||
| const flagHandlers = { | ||
| l: lFlag, | ||
| w: wFlag, | ||
| c: cFlag, | ||
| }; | ||
|
|
||
| const metrics = ["lineCount", "wordCount", "byteSize"]; | ||
| const fileNames = userArguments.filter((argument) => !argument.startsWith("-")); | ||
| const allFilesData = []; | ||
|
|
||
| extractFilesData(); | ||
| addTotals(); | ||
| const outputData = structuredClone(allFilesData); | ||
|
|
||
| let deleted = false; | ||
| executeFlags(); | ||
| printOutput(); | ||
|
|
||
| function extractFilesData() { | ||
| fileNames.forEach((fileName) => { | ||
| const fileData = {}; | ||
| fileData.name = fileName; | ||
| fileData.text = readFile(fileName); | ||
| fileData.lineCount = calculateLineCount(fileData.text); | ||
| fileData.wordCount = calculateWordCount(fileData.text); | ||
| fileData.byteSize = readByteSize(fileName); | ||
|
|
||
| allFilesData.push(fileData); | ||
| }); | ||
| } | ||
|
|
||
| function addTotals() { | ||
| if (allFilesData.length == 1) { | ||
| return; | ||
| } | ||
| const totalsData = { name: "total" }; | ||
| metrics.forEach((metric) => { | ||
| let sum = 0; | ||
| allFilesData.forEach((file) => { | ||
| sum += file[metric]; | ||
| }); | ||
| totalsData[metric] = sum; | ||
| }); | ||
| allFilesData.push(totalsData); | ||
| } | ||
|
|
||
| function readFile(fileName) { | ||
| const filePath = currentWorkingDirectory + fileName; | ||
| return fs.readFileSync(filePath, "utf8").trimEnd(); | ||
| } | ||
|
|
||
| function calculateLineCount(text) { | ||
| return text.split("\n").length; | ||
| } | ||
|
|
||
| function calculateWordCount(text) { | ||
| return text.split(/\s+/).length; | ||
| } | ||
|
|
||
| function readByteSize(fileName) { | ||
| return fs.statSync(currentWorkingDirectory + fileName).size; | ||
| } | ||
|
|
||
| function executeFlags() { | ||
| for (const flag of flags) { | ||
| if (flagHandlers[flag]) { | ||
| allFilesContents = flagHandlers[flag](); | ||
| } else { | ||
| console.error(`wc: illegal option -- ${flag}\nusage: wc [-Lclmw] [file ...]`); | ||
| process.exit(1); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function lFlag() { | ||
| deleteOutputs(); | ||
| allFilesData.forEach((sourceFile) => { | ||
| const targetFile = outputData.find((file) => file.name === sourceFile.name); | ||
| targetFile.lineCount = getIfTrue(sourceFile, "lineCount"); | ||
| }); | ||
| } | ||
|
|
||
| function wFlag() { | ||
| deleteOutputs(); | ||
| allFilesData.forEach((sourceFile) => { | ||
| const targetFile = outputData.find((file) => file.name === sourceFile.name); | ||
| targetFile.wordCount = getIfTrue(sourceFile, "wordCount"); | ||
| }); | ||
| } | ||
|
|
||
| function cFlag() { | ||
| deleteOutputs(); | ||
| allFilesData.forEach((sourceFile) => { | ||
| const targetFile = outputData.find((file) => file.name === sourceFile.name); | ||
| targetFile.byteSize = getIfTrue(sourceFile, "byteSize"); | ||
| }); | ||
| } | ||
|
|
||
| function deleteOutputs() { | ||
| if (!deleted) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain your thinking that led to a design where you need to manually handle your resources like this?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it was a little convoluted... Given no flags displays all data, but the presence of any flags means only the flagged data is displayed. This would be the same for all flags. I've decided to create a separate array which lists displayed metrics from flags to simplify this. I've also refactored a lot. |
||
| outputData.forEach((file) => { | ||
| metrics.forEach((metric) => { | ||
| delete file[metric]; | ||
| }); | ||
| }); | ||
| } | ||
| deleted = true; | ||
| } | ||
|
|
||
| function printOutput() { | ||
| outputs = []; | ||
| outputData.forEach((file) => { | ||
| let outputString = ""; | ||
| metrics.forEach((metric) => { | ||
| outputString += getIfTrue(file, metric); | ||
| }); | ||
| outputString += ` ${file.name}`; | ||
| console.log(outputString); | ||
| }); | ||
| } | ||
|
|
||
| function getIfTrue(file, metric) { | ||
| if (file[metric]) { | ||
| return String(file[metric]).padStart(8, " "); | ||
| } | ||
| return ""; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A map is typically used to change one array into another using a function that doesn't have side effects. Here, you are changing a lineNumber variable each time the map runs. This isn't wrong, but just be careful of side effects, especially if you ever want to run things asynchronously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see - I have refactored my code, so now I use a foreach to mutate the original array rather than mapping a new one.