London | 26-ITP-Sep | Abdennour Hachemi | Sprint 3 | Coursework3 - #1591
AbdennourHachemi wants to merge 23 commits into
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
3-to-pounds.js works well. You tried many inputs, including "0P" and a negative amount.
Four things before I can mark this Complete:
1-key-errors/1.jshas no changes yet. Please do this exercise too: predict, explain and fix.- Your fixed code is in comments, so it never runs. See my comment on
1-key-errors/0.jsline 15. The same is true in1-key-errors/2.jsand in2-mandatory-debug/1.jsand2.js. In2-mandatory-debug/0.jsboth versions are in comments, so the file prints nothing. time-format.jsb): see my comment on line 32.1-bmi.js: see my comment on line 19.
Add the Needs Review label again once you've pushed.
| // =============> write your new code here | ||
| // =============> write your explanation here SyntaxError: Identifier 'str' has already been declared => line 10 varibale str should not be declared again. | ||
| // =============> write your new code | ||
| // function capitalise(str) { |
There was a problem hiding this comment.
Your new code on lines 15 to 19 is in comments. So it never runs. The original code on lines 8 to 11 still runs. Run this file with node. What do you see? The broken code should stop running, and your fix should run. Can you swap them?
There was a problem hiding this comment.
Fixed now. Your fix runs, and the original is in comments. Good.
| // b) What is the value assigned to num when pad is called for the first time? | ||
| // =============> write your answer here | ||
| // b) What is the value assigned to num when pad is called for the first time?: | ||
| // =============> write your answer here: ***Answer*** : The value assigned to num =61. |
There was a problem hiding this comment.
Line 16 calls pad three times. Which call is first? When the input is 61, what is totalHours? Your log on line 6 prints 00 first. Does that match 61?
There was a problem hiding this comment.
b) on line 31 still says 61.
Look at line 15. The first call is pad(totalHours). So on the first call, num is the value of totalHours, not 61.
61 seconds is 1 minute and 1 second. How many whole hours is that? Your answer to c) says the first call returns "00". Which number gives "00"?
There was a problem hiding this comment.
Nearly. On the first call, num is the number 0. "00" is what pad gives back, which is your answer to c). The value going in is a number, and the value coming out is a string.
Please change b) on line 31 to 0.
|
|
||
| function calculateBMI(weight, height) { | ||
| // return the BMI of someone based off their weight and height | ||
| return weight / (height * height); |
There was a problem hiding this comment.
Line 15 says the function should return a string to 1 decimal place. Your toFixed(1) is on line 22, outside the function. What does calculateBMI(70, 1.73) return on its own?
There was a problem hiding this comment.
It returns the string "23.4" now. Good.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Most of the list is done, thanks. The key-errors files and 2-mandatory-debug/0.js now run your fix, and calculateBMI returns a string.
Three things before I can mark this Complete:
2-mandatory-debug/1.jsand2.js: the fix is still in comments. See my comment on1.jsline 15.time-format.jsb): see my reply on line 31.1-key-errors/1.jsfails Prettier now. Lines 21 to 28 have extra spaces at the start. Right click in the file, choose Format Document, and pick Prettier if VS Code asks. To format every time you save, follow the steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md
Add the Needs Review label again once you've pushed.
| // Finally, correct the code to fix the problem | ||
| // =============> write your new code here | ||
|
|
||
| // function sum(a, b) { |
There was a problem hiding this comment.
Your new code on lines 15 to 17 is still in comments, so it never runs. The original code on lines 4 to 9 still runs. 2.js is the same: lines 27 to 32 are in comments, and lines 6 to 14 still run.
Please do what you did in 0.js: put the original code in comments, and take your new code out of them.
To check, right click the file in VS Code's file list and choose Open in Integrated Terminal. Then run node 1.js. It should print The sum of 10 and 32 is 42. For node 2.js, the last digits should be 2, 5 and 6.
There was a problem hiding this comment.
Both files run your fix now. Good.
Prettier still flags 1.js: there is no empty line at the end of the file. Right click in the file, choose Format Document, save and push. To have Prettier do this every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md
There was a problem hiding this comment.
1.js passes Prettier now. Good.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Nearly there. Every file runs your fix now, and most of the list from last time is done.
Three things before I can mark this Complete:
2-mandatory-debug/2.jsline 6 has an unused variable. See my comment there.time-format.jsb): see my reply on line 31.2-mandatory-debug/1.jsfails Prettier. See my reply on line 15.
Add the Needs Review label again once you've pushed.
| // =============> Write your prediction here | ||
| // =============> Write your prediction here: This code will throw an error because the function getLastDigit does not take any parameters but we are passing a parameter to it in the console.log statements. | ||
|
|
||
| const num = 103; |
There was a problem hiding this comment.
const num = 103; on line 6 is not used any more. Your function on line 27 has its own num parameter, so it never reads this one.
You said this yourself on line 35:
The first declaration of num as constent should be removed
But the line is still there. The CYF style guide says the same thing. You ticked it in your PR checklist ("My changes follow the style guide"). Under "Don't leave unused variables" it says:
You should remove any variables that are unused. This is because if you (or someone else) is reading your code, it can be confusing if you see a variable and then find out later that it isn't used. It could make you think that there's a bug, because the variable must have been put there for a reason!
https://curriculum.codeyourfuture.io/guides/reviewing/style-guide/#dont-leave-unused-variables
Please delete line 6. Then run node 2.js again to check it still prints 2, 5 and 6.
There was a problem hiding this comment.
It is in comments now, with the rest of the original code. node 2.js still prints 2, 5 and 6. Good.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Everything on the list is done. Marking this Complete.
Some things to watch for in the next sprint:
- Run every file with node before you push. In this PR, your fixed code was in comments at first, so it never ran. If you run the file, you see this at once.
- Remove what you don't use. If your new code does not need a variable, delete it.
- Turn on format on save. Then Prettier formats each file for you. Right now
time-format.jsline 2 has no;at the end, so Prettier flags it. Here are the steps: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md

Task code
CYF-1053.