Glasgow | 25-ITP-Sep | Christelle Boten | Sprint 2 | Coursework Sprint 2 #852
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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
Christelle-b
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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
3 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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
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. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Can you revert the changes made in Sprint-1 folder to keep this PR branch clean?
| function convertToPercentage(decimalNumber) { | ||
| const decimalNumber = 0.5; | ||
| const percentage = `${decimalNumber * 100}%`; | ||
| const decimalNumber1 = 0.5; | ||
| const percentage = `${decimalNumber1 * 100}%`; | ||
|
|
||
| return percentage; | ||
| } |
There was a problem hiding this comment.
Your function will always return "50%", which is not very useful. Can you improve its reusability so that it can be used to convert any number to an equivalent string in percentage format?
| let BMI = divide.toFixed(2); | ||
| return BMI; |
There was a problem hiding this comment.
What type of value do you expect your function to return? A number or a string?
Does your function return the type of value you expect?
Different types of values may appear identical in the console output, but they are represented and treated differently in the program. For example,
console.log(123); // Output 123
console.log("123"); // Output 123
// Treated differently in the program
let sum1 = 123 + 100; // Evaluate to 223 -- a number
let sum 2 = "123" + 100; // Evaluate to "123100" -- a string.Did you notice the variables squareHeight 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;Then look up the naming conventions in JavaScript, in particulars,
- Variable and function names
- Class and Types names
- Named constants
And update the variable names according to those conventions.
There was a problem hiding this comment.
I have updated my code. My initial code is was returning a string as the method .toFixed() returns a String. Now I have used the function "Number" to convert any possible String to an integer.
Also, in JavaScript, we use camelCase to declare variables and functions. Pascal case is used for classes and types. Lastly, UPPER_SNAKE_CASE are used for Named constants declarations.
cjyuan
added
Reviewed
Christelle-b
force-pushed
the
coursework/sprint-2
branch
from
c5f6681 to
5dc9452
Compare
Christelle-b
added
Needs Review
| // Finally, correct the code to fix the problem | ||
| // =============> write your new code here | ||
| function convertToPercentage(decimalNumber) { | ||
| const percentage = `${(decimalNumber * 100).toFixed(2)}`; |
| } | ||
| function calculateBmi(weight, height) { | ||
| const squareHeight = height * height; | ||
| const divide = weight / squareHeight; |
There was a problem hiding this comment.
I think bmi in lowercase is a more meaningful name than divide.
cjyuan
removed
the
Needs Review
cjyuan
added
the
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
My pull request is about Sprint 2
Questions
N/A