NorthWest Class 4 - Anza Azam - JavaScript - Week 1 - #104
Conversation
|
Your Details |
| return "word".length(); | ||
|
|
||
| var noSpaceString = word.trim(); | ||
| return noSpaceString.length; |
There was a problem hiding this comment.
I like the extra work you've done on this - word.length would have been enough to pass the test, but adding trim is a good idea. When you come to writing tests yourself, think about how you could write a test that would cover this extra functionality.
| //This getRandomNumber() gives a random number between 0 and 10 | ||
| function getRandomNumber() { | ||
| return Math.random() * 10; | ||
| return Math.random() * 10; //Math.random will return a number between 0 and 1 |
There was a problem hiding this comment.
Is Math.random inclusive of 1? And consequently is your method inclusive of 10? Check the definition here. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/random
There was a problem hiding this comment.
learned now that this will not be inclusive of 1 or 10 so we have to add 1 to make these numbers inclusive, thanks really helped me.
|
|
||
| var firstWords = combine2Words(firstWord, " "); | ||
| var secondWords = combine2Words(secondWord, " "); | ||
| return firstWords + secondWords + thirdWord; |
There was a problem hiding this comment.
This works, but you could do it in one line - .concat can take any number of parameters.
| function calculateSalesTax(price) { | ||
| var newPrice = price + (20 / 100) * price; | ||
| return newPrice; | ||
| } |
There was a problem hiding this comment.
This works fine. One change you could make is to declare 20, or 20/100 as a variable called taxRate or something similar, which would make it easier to understand what the code is doing. However, your method has the advantage of being more concise, so it's a matter of taste really, both are correct.
| function convertToBRL(price) { | ||
| var newPrice = (price * 99) / 100; | ||
| var convertedBRL = (newPrice * 5.7).toFixed(2); | ||
| return parseFloat(convertedBRL); |
There was a problem hiding this comment.
Maybe assign 5.7 to a variable called something like exchangeRate, just to make the code more understandable
| let goodCode = | ||
| let goodCode = add(startingValue, 10); | ||
| goodCode = multiply(goodCode, startingValue); | ||
| goodCode = format(goodCode); |
There was a problem hiding this comment.
This is much better. I think it could be even easier to understand if you declare a new variable for each line, describing what had been done, i.e. valueAfterAddition. But this is just a personal preference, your way is fine
| //Write your code in here | ||
|
|
||
| console.log("The ball has shaken!"); | ||
| return answers[Math.floor(Math.random() * 20)]; |
There was a problem hiding this comment.
You could use answers.length here instead of 20, then you won't need to update the number if you add or remove anything from the array
| status = "negative"; | ||
| } else if (index >= 15 && index < 20) { | ||
| status = "very negative"; | ||
| } |
There was a problem hiding this comment.
Do you need 2 parts to your if statements in lines 144, 146? Have you already established one of them? Do you need the check on line 148 at all?
|
Your coursework submission has been closed because nobody has interacted with it in six weeks. You are welcome to re-open it to get more feedback. |
Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in
HOW_TO_MARK.mdin the root of this repositoryYour Details
Homework Details
Notes
What did you find easy?
What did you find hard?
What do you still not understand?
Any other notes?