London | 26-ITP-May | Damilola Odumosu | Sprint 2 | Coursework#1465
London | 26-ITP-May | Damilola Odumosu | Sprint 2 | Coursework#1465d-odumosu wants to merge 14 commits into
Conversation
d-odumosu
added
📅 Sprint 2
Liam310
added
Review in progress
| function capitalise(str){ | ||
| str = `${str[0].toUpperCase()}${str.slice(0)}`; | ||
| return str | ||
| } |
There was a problem hiding this comment.
Generally we reassign the value of a variable if we want to update the value whilst the recognising that what the value represents has stayed the same. Here, you're reassigning str to be something that represents a different thing. Considering this, can you think of an alternative approach to reassigning the existing str variable that would make this a little bit more readable?
There was a problem hiding this comment.
Thank you, i have declared a new variable to hold the capitalised string instead of reassinging str to equal capitalised str
Liam310
added
Reviewed
|
Great work overall, just a couple of small comments! |
d-odumosu
added
Needs Review
| function convertToPercentage(decimalNumber) { | ||
| decimalNumber = 0.5; | ||
| const percentage = `${decimalNumber * 100}%`; | ||
| return percentage; |
There was a problem hiding this comment.
It looks like when you fixed the explanation above (removing the bit about const variables), you accidentally changed this function too! The function worked well before. What's the problem with it in this form?
Liam310
added
Reviewed
2 participants
Self checklist
Changelist
completed all mandatory and stretch