Repository navigation
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.
There was a problem hiding this comment.
None of the five console.asserts fail when I run it, so we can be confident toPounds works. These are good tests, and that will come in handy in your current coursework on testing. I also like the care you took over the failure messages: showing the current and target output side by side makes a failure easy to understand straight away.
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.
abdishakoor-dev
left a comment
There was a problem hiding this comment.
That's everything. Marking this Complete, well done.

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