London | 26-ITP-Sep| Abdennour Hachemi| Sprint 2 | Coursework - #1547
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.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Nice work, this is thorough. The table of function calls in 1-percentage-change.js is a good way to answer a), all five are right, and 2-mandatory-errors/3.js is a model answer: a prediction, the exact error, and three different ways to fix it.
A few things to fix:
-
.gitignoreis in this PR and it is not part of the task. See my comment. -
1-key-exercises/3-paths.jsline 20, see my comment. -
1-key-exercises/4-random.js, see my comment. -
2-mandatory-errors/1.js, see my comment. -
3-mandatory-interpret/2-time-format.jsanswer f), see my comment. -
Could you run Prettier on the files you have changed? Eleven of them are not formatted at the moment.
Add the Needs Review label again when you have pushed.
| .DS_Store | ||
| .vscode | ||
| **/.DS_Store No newline at end of file | ||
| **/.DS_Storeeducation-blog/education-blog |
There was a problem hiding this comment.
Two things here. This file is not part of the Sprint 2 task, which is what the bot at the top of the page is complaining about. Run git checkout main -- .gitignore to put it back, then commit and push.
Also worth seeing what happened: your new entry got joined onto the end of the previous line, so the file now reads **/.DS_Storeeducation-blog/education-blog as a single pattern. That matches nothing, so **/.DS_Store has stopped working too.
There was a problem hiding this comment.
Sorted, .gitignore matches main again and the bot is happy.
|
|
||
| const dir = ; | ||
| const ext = ; | ||
| const dir = filePath.slice(1,lastSlashIndex); |
There was a problem hiding this comment.
Run this file and read the dir line. It prints Users/mitch/... but the path starts with a /, so the first character is missing. What is the 1 doing as the first argument to slice, and what would happen if it were 0?
There was a problem hiding this comment.
That's the one. dir prints the whole path now.
| test = test+ minimum; | ||
| console.log("test+ minimum" , test) | ||
|
|
||
| // Last operation is to add 1 to num 1 is stored in a variable called minimum |
There was a problem hiding this comment.
This line says 1 is stored in minimum, but you changed line 1 to const minimum = 10, so the two no longer agree.
There is something else worth checking here. Look at the order you do the steps in. On line 28 you add minimum, then on line 33 you use Math.floor. Now look at line 4: which of those two happens first there? Does the order change the answer?
And your comments walk through the steps well, but they never say what num ends up being. What is the smallest value it can have, and the largest?
There was a problem hiding this comment.
The order is right now and you have said what num ends up being. One thing left on the last step, see my new comment on line 27.
| // trying to create an age variable and then reassign the value by 1 | ||
|
|
||
| const age = 33; | ||
| let age = 33; |
There was a problem hiding this comment.
The fix is right and the file runs. This section also asks you to interpret the error and explain why it happened, and there is nothing written down here. You did that well in 2.js, 3.js and 4.js, so the same again. What did node print before you changed const to let?
There was a problem hiding this comment.
Good, the message and the reason are both written down now.
| /* movieLength = 60 => exactMovieLength = 0:1:0 */ | ||
| /* movieLength = 3676 => exactMovieLength = 1:1:16 */ | ||
| /* */ | ||
| /* This code work for all values of movieLength provided that it doesn't exeed 86400 which is 24 hours = 1 day (realisticly no movie is over 24 hours) */ |
There was a problem hiding this comment.
Your three examples above are all correct, I ran them. This conclusion is not right though. Try 90000 and see what you get; going over 24 hours is fine.
Look again at your own 60 => 0:1:0 example. Is that how a clock shows one minute? And try a negative number, and one with a decimal like 90.5.
There was a problem hiding this comment.
Much better. I ran -3600 and 90000 and got what you wrote.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
That's four of the five sorted, thanks. The f) answer in 2-time-format.js is a good one now, it names four separate things that break and your two new examples are both right.
Two things left:
-
1-key-exercises/4-random.jsline 27, see my comment. -
The formatting. "My code is consistently formatted" is on the README checklist, and the tool that does it for you is called Prettier. It rearranges spacing and indentation to one agreed style, so your code is easy to read and so a reviewer only sees the changes you meant to make. Eleven of your files still fail that check.
Prettier comes with the CYF extension pack you were asked to install during onboarding. If you are not sure you have it, open VS Code, go to Extensions, and search for CodeYourFuture Extension Pack. Install it if it is not there: https://marketplace.visualstudio.com/items?itemName=CodeYourFuture.cyf-extension-pack
Then open each file in
Sprint-2, right click in the editor, choose Format Document, and pick Prettier if VS Code asks which formatter to use. Save, commit the changes it makes, and push. To make this happen every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md
Add the Needs Review label again once you have pushed.
|
|
||
|
|
||
|
|
||
| // Last operation is to add 1 to hit the maximum value (since Math.random() never quite reaches 1). This ensures all 100 numbers (1 through 100) are equally reachable.eg 39 => 40 |
There was a problem hiding this comment.
Nearly. The order of your four steps is right now. But the last step in line 4 is + minimum, not + 1. Here minimum is 1, so the two look the same. What would this step add if minimum were 10?
There is a + 1 in line 4 as well, inside (maximum - minimum + 1). Which of the two makes 100 reachable?

Self checklist
Task code
CYF-1039
A solution to all exercies in Sprint 2. Thanks for reviwing it!