Manchester | 26-ITP-Sept | Fatima Rouchi | Sprint 3 | Coursework/sprint 3 - #1634
fatimarouchi wants to merge 19 commits into
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
fatimarouchi
added
the
Needs Review
iscmiguelsamaniego
left a comment
iscmiguelsamaniego
left a comment
There was a problem hiding this comment.
"Fantastic work overall across all these exercises! You've shown great progress in fixing scope issues, handling function parameters, working with string methods, and writing robust tests.
As you keep practicing, what strategies do you find most helpful for anticipating edge cases before writing your code?"
There was a problem hiding this comment.
Spot on! This is a clever way to handle string slicing and padding to format the currency correctly.
- What happens if someone passes
"599"without the trailing"p", or if they pass a number like599directly? How could you make the function flexible enough to handle those cases?
There was a problem hiding this comment.
Without the p, it removes a digit by mistake. I can convert the input to a string and remove the p only when it’s there.
fatimarouchi
added
Needs Review
mayowaolokun
added
Review in progress
mayowaolokun
left a comment
mayowaolokun
left a comment
There was a problem hiding this comment.
You have clearly understood the main ideas, and your fixes and edge-case tests show good thinking.
I have left a few comments on things to look at again, mainly output formatting, adding more test examples, and how your tests report success. Please read through them, make your changes.
| return percentage; | ||
| } | ||
|
|
||
| console.log(convertToPercentage(0.5)); |
There was a problem hiding this comment.
Well done fixing the original error by removing the redeclared const decimalNumber, and your code runs correctly; just note that toFixed(1) changes the output from 50% to 50.0%, so either remove it to match the expected result or add a short comment explaining why you chose to include it.
| // return the BMI of someone based off their weight and height | ||
| return (weight / (height * height)).toFixed(1); | ||
| } | ||
| console.log(calculateBMI(52, 1.63)); |
There was a problem hiding this comment.
The exercise asks for the function to be implemented and checked with several inputs, ensuring the function works across multiple inputs. You can add a few more test checks to demonstrate this.
| return words.toUpperCase().trim().split(/\s+/).join("_"); | ||
| } | ||
|
|
||
| console.log(toUpperSnakeCase("hello there")); |
There was a problem hiding this comment.
Good work, your function correctly converts the string to UPPER_SNAKE_CASE, and using trim() with split(/\s+/) is a thoughtful way to handle extra spaces; you can add a few more test examples, such as a single word like "hello", and a string with extra spaces to show it works for a range of inputs
| } | ||
|
|
||
| return `${formattedHours}:${minutes} ${suffix}`; | ||
| } |
There was a problem hiding this comment.
Nice work using hours % 12 with the reset to 12, which handles both midnight and noon correctly; one thing to fix is that formattedHours is a number, so 08:00 returns 8:00 am instead of 08:00 am. You look into fixing that.
|
|
||
| const noonOutput = formatAs12HourClock("12:00"); | ||
| const noonTarget = "12:00 pm"; | ||
| console.assert( | ||
| noonOutput === noonTarget, | ||
| `current output: ${noonOutput}, target output: ${noonTarget}`, | ||
| ); | ||
|
|
||
| const midnightOutput = formatAs12HourClock("00:00"); | ||
| const midnightTarget = "12:00 am"; | ||
| console.assert( | ||
| midnightOutput === midnightTarget, | ||
| `current output: ${midnightOutput}, target output: ${midnightTarget}`, | ||
| ); | ||
|
|
||
| const singleDigitMinuteOutput = formatAs12HourClock("9:5"); | ||
| const singleDigitMinuteTarget = "9:05 am"; | ||
| console.assert( | ||
| singleDigitMinuteOutput === singleDigitMinuteTarget, | ||
| `current output: ${singleDigitMinuteOutput}, target output: ${singleDigitMinuteTarget}`, | ||
| ); | ||
|
|
||
| console.log("All formatAs12HourClock tests passed."); |
There was a problem hiding this comment.
Great job testing noon and midnight, which are the edge cases most likely to break this function. Two things to think about: what happens to your final success message if one of the assertions fails, and is "9:5" a valid 24-hour time whose expected output matches the format of your other tests?
mayowaolokun
added
Reviewed
3 participants
Self checklist
CYF-1053