NW | 25-ITP-Sep | Ahmad Hmedan | Sprint 2 | Sprint 2 Coursework #783
NW | 25-ITP-Sep | Ahmad Hmedan | Sprint 2 | Sprint 2 Coursework #783AhmadHmedann wants to merge 10 commits intoCodeYourFuture:mainfrom
Conversation
AhmadHmedann
added
the
Needs Review
Sprint-2/1-key-errors/0.js
Outdated
| // =============> write your new code here | ||
| // =============> As I predict it will throw a SyntaxError: Identifier 'str' has already been declared | ||
| //because in JavaScript we can not redeclare the same variable in the same scope. | ||
| function Capitalise(str){ |
There was a problem hiding this comment.
In Javascript, we follow the convention of camelCase for functions and variables
| function calculateBMI(weight, height) { | ||
| // return the BMI of someone based off their weight and height | ||
| } No newline at end of file | ||
| let BMI=weight/(height*height); |
There was a problem hiding this comment.
Nice! Even cleaner - removing the variable entirely if we just return it immediately
| function ConvertToUpperSnakeCase(s1) | ||
| { | ||
| let result = ""; | ||
| for(let i=0;i<s1.length;i++) |
There was a problem hiding this comment.
Is there a standard library function we could use here?
| else | ||
| result+=s1[i]; | ||
| } | ||
| let UPPER_SNAKE_CASE=result.toUpperCase(); |
There was a problem hiding this comment.
We typically follow the convention of camelCase for Javascript variables
| // You should call this function a number of times to check it works for different inputs | ||
|
|
||
|
|
||
| function toPound(penceString) |
There was a problem hiding this comment.
This is functionally looking good 🙂
I would suggest using VS Code's format document feature - this will ensure your Javascript code is following a standard format. This makes it easier for yourself, and others, to read the code in the future 🙂
https://code.visualstudio.com/docs/editing/codebasics#_formatting
| currentTime = TimeAs12hours("00:00"); | ||
| targetTime = "12:00 am"; | ||
| console.assert( | ||
| currentTime === targetTime, | ||
| `current time:${currentTime}, Target Time:${targetTime}` | ||
| ); No newline at end of file |
There was a problem hiding this comment.
I get the following assertion error on this test case:
Assertion failed: current output: 12:00 am, target output: 12:00 pm
There was a problem hiding this comment.
Thank you very much for your feedback, it was so clear and helpful. For 5-stretch-extend/format-time The Task was: "Your task is to write tests for as many different groups of input data or edge cases as you can, and fix any bugs you find." I added two assertion and both were failed so I rewrite the code,
There was a problem hiding this comment.
This is a really great submission! A lot of my comments were "nitpicks", but you've cleaned your code up really well
Will mark as complete
There was a problem hiding this comment.
Thank you so much for your kind feedback! I really appreciate your time and comments — they helped me improve my code quality.
CameronDowner
added
Reviewed
AhmadHmedann
added
Needs Review
CameronDowner
added
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
I have answered all questions in the Sprint 2
Questions
Can you give me feedback on my answer in Sprint2/5-strech-extend folder