-
-
Notifications
You must be signed in to change notification settings - Fork 197
London | 25-ITP-May | Houssam Lahlah | Sprint 2 | Coursework/sprint 2 #700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…dle, and last names.
…om file path string using slice.
…ween minimum and maximum.
Moved padStart(2, "0") to a single location after calculating rawHour. Improves readability and makes the function easier to maintain. Thanks for the helpful suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start on this sprint's tasks, You have solved all of these tasks well. I have spotted a few areas where you could improve code further.
Regarding your question about titles in the PR - I'm not sure I understand what you are asking. If you mean the title format, follow the instructions in any errors that get notified. If there are no notes, it is formatted correctly.
Sprint-2/1-key-errors/0.js
Outdated
// =============> write your new code here | ||
// =============> write your new code here | ||
function capitalise(str) { | ||
let capitalisedStr = `${str[0].toUpperCase()}${str.slice(1)}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming the variable allows you to fix the bug, great!
The pattern here is you are defining a variable and then immediately returning it. Can you think how to make this code even simpler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi LonMcGregor,
Thank you for your feedback.
I see what you mean — I don’t actually need the extra variable here.
I can simplify it by returning the string directly, Like this :
function capitalise(str) {
return ${str[0].toUpperCase()}${str.slice(1)}
;
}
because this way the function is shorter and still clear.
Sprint-2/1-key-errors/1.js
Outdated
|
||
function convertToPercentage(decimalNumber) { | ||
const percentage = `${decimalNumber * 100}%`; | ||
return percentage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same comment as in task 0 - this works, but could be simplified further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi LonMcGregor,
Thank you! I see your point — the extra variable isn’t needed here either. I can simplify it by returning the result directly, like:
function convertToPercentage(decimalNumber) {
return ${decimalNumber * 100}%
;
}
This makes it cleaner while keeping the same functionality.
Self checklist
Changelist
Questions