london-10/iryna-lypnyk/completed-mandatory&extra-exercises#522
london-10/iryna-lypnyk/completed-mandatory&extra-exercises#522IrynaLypnyk wants to merge 4 commits intoCodeYourFuture:masterfrom
Conversation
Schboostie
left a comment
There was a problem hiding this comment.
Hi Iryna, I really liked the way you tackled alot of the problems. I hope you found my comments useful🌟
| function introduceMe(name, age) | ||
| return `Hello, my {name}` is "and I am $age years old`; | ||
| function introduceMe(name, age) { | ||
| return `Hello, my name is ${name} and I am ${age} years old`; |
There was a problem hiding this comment.
Good use of back tick when adding variables to a string.
| @@ -1,16 +1,20 @@ | |||
| // Add comments to explain what this function does. You're meant to use Google! | |||
| // Returns a random integer from 0 to 9 | |||
There was a problem hiding this comment.
You could add further comments to explain how Math.random() returns a decimal value which is then scaled to a desired range.
There was a problem hiding this comment.
Agreed. It'll be helpful for collaborators to know what the random operator does.
Also, is 9 the highest output of this function? What if math.random selects 0.9, for example?
There was a problem hiding this comment.
Oh, yes, you are right. Math.random() returns a decimal value, and it can be scaled more than just to 9. But if to speak just about my function - it returns form 0 to 9. Thank you, Shahid and Katie!
mandatory/4-tax.js
Outdated
|
|
||
| function calculateSalesTax() {} | ||
| function calculateSalesTax(price) { | ||
| return price + price/5; |
There was a problem hiding this comment.
It can be useful to declare separate variables at stages of each operation. When calculations become more complexed it may be necessary to check various stages to see if and when mistakes occur.
|
|
||
| function addTaxAndFormatCurrency() {} | ||
| function addTaxAndFormatCurrency(number) { | ||
| let tax = calculateSalesTax(number); |
There was a problem hiding this comment.
I really like the use of the tax variable to break down the stages of the function.
extra/2-piping.js
Outdated
|
|
||
| // Why can this code be seen as bad practice? Comment your answer. | ||
| let badCode = | ||
| let badCode = `£${(startingValue + 10 ) * 2}` |
There was a problem hiding this comment.
I can not see a comment to explain why this is bad code. Did you add one?
There was a problem hiding this comment.
This could use a little work, as the exercise above asks you to use the functions (like you have below in goodCode). Why would all your functions on one line (like you have in goodCode) be bad? Hint - bugs, readability
There was a problem hiding this comment.
I concentrated so much on JavaScript that I didn`t notice task "Comment your answer..." ))) I updated my code and pushed it
| // Why can this code be seen as bad practice? Comment your answer. | ||
| let badCode = | ||
| let badCode = `£${(startingValue + 10 ) * 2}` | ||
|
|
There was a problem hiding this comment.
My explanation was:
//Multiple methods are used in one line which makes it hard to determine the results from each methods execution. It is confusing and hard to follow.
|
Iryna, well done this is a fantastic effort! Really impressed with your work on the extra exercises too. Keep it up! |
|
|
||
| return "The total is total"; | ||
| let total = a + b; | ||
| return `The total is ${total}`; |
| a * b * c; | ||
| return; | ||
| return a * b * c; | ||
| } |
There was a problem hiding this comment.
All functions looking good here, Iryna!
| @@ -1,16 +1,20 @@ | |||
| // Add comments to explain what this function does. You're meant to use Google! | |||
| // Returns a random integer from 0 to 9 | |||
There was a problem hiding this comment.
Agreed. It'll be helpful for collaborators to know what the random operator does.
Also, is 9 the highest output of this function? What if math.random selects 0.9, for example?
| // Write the body of this function to concatenate three words together. | ||
| // Look at the test case below to understand what this function is expected to return. | ||
|
|
||
| return firstWord.concat(' ', secondWord, ' ', thirdWord); |
| } | ||
|
|
||
| // Add comments to explain what this function does. You're meant to use Google! | ||
| //Concatenate strings,numbers, arrays: |
There was a problem hiding this comment.
Nice start but think we can add a little more detail here. What specifically is this function doing?
Also, concat can be used to concatanate strings and arrays, but can it concatanate numbers?
extra/1-currency-conversion.js
Outdated
|
|
||
| function convertToBRL() {} | ||
| function convertToBRL(price) { | ||
| return Number(((price * 5.7)*0.99).toFixed(2)); |
There was a problem hiding this comment.
This is smart and gets the job done, but it is trying to achieve too much on one line - making it vulnerable to bugs and difficult to read.
How can you take these steps and break them into separate lines?
| function format() { | ||
|
|
||
| function format(num) { | ||
| return `£${num}` |
extra/2-piping.js
Outdated
|
|
||
| // Why can this code be seen as bad practice? Comment your answer. | ||
| let badCode = | ||
| let badCode = `£${(startingValue + 10 ) * 2}` |
There was a problem hiding this comment.
This could use a little work, as the exercise above asks you to use the functions (like you have below in goodCode). Why would all your functions on one line (like you have in goodCode) be bad? Hint - bugs, readability
| // Why can this code be seen as bad practice? Comment your answer. | ||
| let badCode = | ||
| let badCode = `£${(startingValue + 10 ) * 2}` | ||
|
|
extra/2-piping.js
Outdated
| /* BETTER PRACTICE */ | ||
|
|
||
| let goodCode = | ||
| let goodCode = format(multiply(add(startingValue, 10), 2)); |
There was a problem hiding this comment.
This does the job, but good code is highly readable and easy to modify so tends to be broken out into multiple lines. Can you break this out into steps?
| let allAnswers = [veryPositiveAnswers, positiveAnswers, negativeAnswers, veryNegativeAnswers]; | ||
|
|
||
| function findRandomIndex(max){ | ||
| return Math.floor(Math.random() * max); |
| console.log("The ball has shaken!"); | ||
| let num1 = findRandomIndex(4); | ||
| let num2 = findRandomIndex(5); | ||
| return allAnswers[num1][num2]; |
There was a problem hiding this comment.
Why are you returning two indices here?
There was a problem hiding this comment.
Ah, I think I see. Do you need to call findRandomIndex within the shakeBall function so that it changes the output each time?
|
|
||
| } | ||
|
|
||
| function checkAnswer(answer) { |
There was a problem hiding this comment.
I am not sure what this is doing (am new to javascript). Could you please explain? Remember to focus on readability. Looks great though and it's awesome to see all the tests passing.
No description provided.