-
-
Notifications
You must be signed in to change notification settings - Fork 125
Manchester | Zabihollah Namazi | Module-Data-Flows | challenges | CowSay two #102
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
Conversation
|
||
async function fetchData() { | ||
try{ | ||
const response = await fetch("https://xkcd.now.sh/?comic=latest") |
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.
Please, add closing semicolon at the end of your expressions. Though your JS will work, it prevents you from unexpected errors.
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.
thanks
|
||
} | ||
|
||
fetchData() |
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.
Closing semicolon here please.
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.
ok thanks
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.
Good work. I left you some comments and suggestions.
const readLine = require('node:readline'); | ||
const reader = readLine.createInterface({ | ||
input: process.stdin, | ||
output: process.stdout | ||
}); |
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 think in solution1.js
, the program is expected to use the command line argument as input, so that one can run the code with an input string "Moo moo" as
node solution1.js "Moo moo"
The given link "Accept an argument" in readm.md
is broken, but you can find similar guide from https://www.30secondsofcode.org/js/s/command-line-arguments/ (or use ChatGPT).
readline
is for solution2.js
.
// how will you account for the parameter being empty? | ||
|
||
} | ||
|
||
//4. Pipe argument into cowsay function and return a cow | ||
|
||
// how will you log this to the console? | ||
// node solution1.js |
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 think the expectation are
- In (3),
cowSay(saying)
should return a string that represents the cow saying the value of parametersaying
. - In (4), output the cow to the console as
console.log(cowSay(...))
where...
is the command line argument.
let topLine = "_"; | ||
let bottomLine = "-"; | ||
let sayingWord = ``; | ||
let cowAscii = ` |
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.
It is better to
- declare variables used only inside a function as local variables in the function
- declare variables that do not change using
const
(e.g.,cowAscii
)
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.
done
Stopped watching this PR for changes. If you will need my review for this PR in the future, please, ping me and add 'Needs review' label. |
Just ignore the fetch commit please .
I have done both solutions in this branch.
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.