London | 26-ITP-Sep | Mahir Shah | Sprint 3 | Sprint 3 Coursework - #1625
MahirShah300 wants to merge 18 commits into
Conversation
…riable with same name
…meter and giving default value
…so properly returned
…um parameter to function getLastDigit
✅ 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.
This comment has been minimized.
This comment has been minimized.
| // =============> write your new code here | ||
|
|
||
| function capitalise(str) { | ||
| str = `${str[0].toUpperCase()}${str.slice(1)}`; |
There was a problem hiding this comment.
A general rule of thumb is - if you overwrite the value of parameter, it could cause bugs down the code as the original value is lost.
could you perhaps create a local variable instead and return that ?
There was a problem hiding this comment.
I made a change to use a variable capitaliseStr
There was a problem hiding this comment.
Good. str keeps its first value now.
| // =============> write your new code here | ||
|
|
||
|
|
||
| function square(num = 3) { |
There was a problem hiding this comment.
this will default the number to 3 if not passed (which is not what the requirement was). Instead, Can you try calling the square with different numbers and see the output
There was a problem hiding this comment.
I removed the default parameter, it squares the number when calling it with different numbers
There was a problem hiding this comment.
Good. square works for any number now.
| // Use the MDN string documentation to help you find a solution | ||
| // This might help https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/toUpperCase | ||
| function toUpperSnakeCase(inputString) { | ||
| return inputString.toUpperCase().replaceAll(" ", "_"); |
There was a problem hiding this comment.
why was the replaceAll used ? does this print exactly what is expected or does it add some additional strings which were not part of input string ?
There was a problem hiding this comment.
replaceAll goes through a string and removes all cases of the first input, and replaces it with the second input. It prints what is expected, and the only thing added are _ in places of " ". If there's any unexpected whitespace " " after a string, they would replaced with _
There was a problem hiding this comment.
Clear answer, thanks.
| } | ||
| function formatAs12HourClock(time) { | ||
| const hours = Number(time.slice(0, 2)); | ||
| if ( |
There was a problem hiding this comment.
good effort. could you think of other scenarios you might have missed ?
-
what happens when you pass empty strings - formatAs12HourClock(" :30")
-
what happens when you pass nothing - formatAs12HourClock()
-
what happens when you pass without semicolon - formatAs12HourClock(1230)
There was a problem hiding this comment.
1.Empty strings are caught and show "Not a valid time"
2. This would cause errors. I fixed it by using default parameter ""
3. Without a semicolon is also caught. However if the input is not a string it would cause errors, which I fixed by checking if the input is type string
There was a problem hiding this comment.
This is the stretch part, so it does not block Complete. But one check: what does formatAs12HourClock(" :30") give now? I get "12:30 am".
There was a problem hiding this comment.
It does give "12:30 am". This is because there is validation on whitespace in the input string. I have added it now
| isNaN(minutes) || | ||
| hours >= 24 || | ||
| hours < 0 || | ||
| minutes >= 60 || |
There was a problem hiding this comment.
you are not checking if he hours and minutes are integers eg. "12:30.5" would pass your tests
There was a problem hiding this comment.
"12:30.5" actually doesn't pass the tests because of my checks for only one "." and one ":", but I added a check to make sure the numbers are integers
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Thanks for the changes. Your answers are clear and correct. In time-format.js e) you explain each step of the while loop. That is very good.
One thing before I can mark this Complete:
3-to-pounds.js: see my comment on line 24.
Add the Needs Review label again once you've pushed.
| .substring(paddedPenceNumberString.length - 2) | ||
| .padEnd(2, "0"); | ||
|
|
||
| console.log(`£${pounds}.${pence}`); |
There was a problem hiding this comment.
Your function prints the price, but it does not give it back. What does toPounds("399p") return?
You explained this yourself in 2-mandatory-debug/0.js: a function that only uses console.log gives back undefined.
Say another part of a program needs the price. For example, it wants to add it to a sentence. Can it get the price from toPounds now? What would you change?
There was a problem hiding this comment.
Not currently. I have changed the console.log to return £${pounds}.${pence} instead
There was a problem hiding this comment.
Your function returns the price now, which is right. And you call it a few times with different values. But how do you know it works each time? Line 6 says to call it "to check it works for different inputs".
There are a few ways to do this:
- Wrap each function call in lines 27-31 in
console.log, so the returned value is printed in the terminal:console.log(toPounds("10p")); - Or use
console.assert, like you did informat-time.js, to compare each result with the price you expect.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Thanks. toPounds gives the price back now, which is the important part.
Two small things before I can mark this Complete, both in 3-to-pounds.js:
- Line 24: see my reply there.
- The file fails Prettier since your last change. Open it, right click, choose Format Document, save, and push.
Add the Needs Review label again once you've pushed.

Self checklist
Task code
CYF-1053
Changelist
Answered the questions, fixed all bugs