-
-
Notifications
You must be signed in to change notification settings - Fork 239
London | 25-ITP-Sep | Payman Issa Baiglu | Sprint 2 | Coursework/sprint 2 #838
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
… correct convertToPercentage function implementation
jennethydyrova
left a comment
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 @PaymanIB! Very good work on these exercises! I left several comments you need to address. You also have some extra file in the commit you pushed, can you please remove it?
Sprint-2/1-key-errors/0.js
Outdated
| // Predict and explain first... | ||
| // =============> write your prediction here | ||
|
|
||
| // it gives us an error. |
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.
You should try to be more specific, what type of error?
| let str1 = `${str[0].toUpperCase()}${str.slice(1)}`; | ||
| return str1; |
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.
Why do you use let here? Also, can you think of a way to simplify these two lines?
| // Why will an error occur when this program runs? yes | ||
| // =============> write your prediction here | ||
| // decimalNumber is declared before and can not be declared again. | ||
| // console.log(decimalNumber) should be console.log(convertToPercentage) |
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.
This is correct but why is that a case? Why we can't do console.log(decimalNumber)?
| // Finally, correct the code to fix the problem | ||
| // =============> write your new code here | ||
| function convertToPercentage(decimalNumber) { | ||
| //const decimalNumber = 0.5; |
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.
Let's keep the code clean and remove all scratch work and commented out code (within functions you wrote).
| 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.
Can you think of a way to simplify this?
| // console.log(`The sum of 10 and 32 is ${sum(10, 32)}`); | ||
|
|
||
| // =============> write your explanation here | ||
| //the return statement is not returning any value. a+b declared after return. |
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.
You are right about the return but be careful with second part, a + b is not not a declaration, it's an expression.
|
|
||
| // Predict the output of the following code: | ||
| // =============> Write your prediction here | ||
| // the result will be 3 times |
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.
What does it mean?
| const penceStringWithoutTrailingP = penceString.substring(0 , penceString.length - 1); | ||
| const paddedPenceNumberString = penceStringWithoutTrailingP.padStart(3, "0"); | ||
| const pounds = paddedPenceNumberString.substring(0,paddedPenceNumberString.length - 2); | ||
| const pence = paddedPenceNumberString.substring(paddedPenceNumberString.length - 2).padEnd(2, "0"); | ||
|
|
||
| return `£${pounds}.${pence}`; |
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.
Do you have prettier installed? If no, I would recommend installing it and formatting your code. This is not a correct indentation, makes your code look messier
| const totalMinutes = (seconds - remainingSeconds) / 60; //1 | ||
| const remainingMinutes = totalMinutes % 60; //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.
Do you need these comments here?
|
Hi @PaymanIB! Do you want to do another round of review? If yes, please change label to needs review and request review. |
in this PR: Sprint 2 Coursework completed. errors fixed and questions answered