Glasgow | 25-ITP-SEP | Mohammed Abdoon | Sprint 2 | Coursework #725
Conversation
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. |
2 similar comments
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. |
M-Abdoon
added
the
Needs Review
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. |
1 similar comment
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. |
M-Abdoon
changed the title
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. |
| let BMI = weight/squaredHeight; | ||
| BMI = BMI.toFixed(1); | ||
| return Number(BMI); |
There was a problem hiding this comment.
Did you notice the variables squaredHeight and BMI are rendered in different colors?
Many IDEs and viewers that support syntax highlighting (including GitHub) display identifiers in different formats and colors. Can you find out why?
let bmi, camelCase;
let Bmi, PascalCase;
let BMI, UPPER_SNAKE_CASE;Can you look up the naming conventions in JavaScript? In particular,
- Variable and function names
- Class and Types names
- Named constants
Then, update the variable names according to those conventions.
| // # The Python visualizer stops working when it runs pad because (padStart is not a function), maybe because it uses ES6 version of javascript! | ||
| // The return value of pad when it's called for the first time is 00 |
There was a problem hiding this comment.
A common convention for indicating that a value is a string is to enclose it in double quotes. For example, "00".
| currentOutput = formatAs12HourClock("00:37"); | ||
| targetOutput = "00:37 am"; |
There was a problem hiding this comment.
Can you look up the 12-hour clock equivalents of the 24-hour times "00:37" and "12:37"?
cjyuan
added
Reviewed
M-Abdoon
added
the
Needs Review
There was a problem hiding this comment.
Changes look good.
Please note that in CYF courses, the recommended way to inform the reviewer of your changes is to do both of the following:
- Reply to their feedback.
- In the responses, clarify how each piece of feedback was addressed to demonstrate that you've carefully reviewed the suggestions.
- You may find the suggestions in this PR Guide useful.
- Your response may trigger a notification (depending on the reviewer's settings), helping ensure they’re aware of the updates you’ve made.
- In the responses, clarify how each piece of feedback was addressed to demonstrate that you've carefully reviewed the suggestions.
- Replace the "Reviewed" label by a "Needs review" label (which you have done -- great!)
- Without this label, the reviewer would not know if your changes is ready to be reviewed.
| let hours = time.slice(0, 2); | ||
| let minutes= time.slice(3, 5); | ||
| let timeIndicator; | ||
|
|
||
| if (Number(hours) >= 12) { | ||
| hours = String(hours - 12); | ||
| hours = hours.padStart(2, "0"); | ||
| timeIndicator = "pm"; | ||
| } else { | ||
| timeIndicator = "am"; | ||
| } | ||
| return `${time} am`; | ||
| if ( hours === "00") | ||
| hours = "12"; | ||
|
|
||
| return `${hours}:${minutes} ${timeIndicator}`; |
There was a problem hiding this comment.
On lines 10-11: Mixing numbers and strings can be dangerous.
A better approach could be:
- Convert
hoursto a number once at the beginning - Carry out calculation using only numbers in the function body (no type conversion needed)
- At line 20, call
.padStart()once to formathoursas a 2-digit string.
cjyuan
added
Complete
Ok, I do that for the next PL reviews, thank you so much. |
2 participants
Learners, PR Template
Self checklist
Changelist
I have completed all the exercises and worked through errors, debugging, implementations, interpretations, and practicing handling time formats.