glasgow class 6-siver omar-javascript-week2 #450
Conversation
There was a problem hiding this comment.
Well done on completing the mandatory exercises, it looks like you're getting to grips with the basic logic.
I've spotted a couple of issues, I'd suggest you take a look through those and if you've got time try and resolve the noted problems. There are a few other notes I've made where your code could be formatted a little better or suggestions of different ways to approach the problems. Take a read through and bear those in mind for future exercises.
| @@ -23,13 +23,13 @@ function getMood() { | |||
| function greaterThan10(num) { | |||
| let isBigEnough; | |||
There was a problem hiding this comment.
Nitpick: What you've done is correct, but it leaves a pointless variable isBigEnough that is never used. One possibility could be to use the isBigEnough variable to store the condition output. e.g.
let isBigEnough = num > 10;
if (isBigEnough) {
...
Or alternatively you could have just deleted the isBigEnough variable.
| function isAcceptableUser(userAge, isLoggedIn) {} | ||
| // npm test -- --testPathPattern 2-function-creation | ||
| function isAcceptableUser(userAge, isLoggedIn) { | ||
| if( userAge >= 18 && isLoggedIn){ |
There was a problem hiding this comment.
Nitpick: While this is OK, it's slightly better to do isLoggedIn === true as this type checks isLoggedIn to make sure it's passed into the function as a boolean.
| @@ -15,26 +23,56 @@ function isAcceptableUser(userAge, isLoggedIn) {} | |||
| is applieds and 142.5 should be returned) | |||
| */ | |||
|
|
|||
| function applyDiscount(totalPrice) {} | |||
| function applyDiscount(totalPrice,discount) { | |||
There was a problem hiding this comment.
Issue: There's no need to take discount as a parameter to this function. You should only define the inputs that are needed by the function or expected by the tests.
| function applyDiscount(totalPrice) {} | ||
| function applyDiscount(totalPrice,discount) { | ||
| if (totalPrice >200) { | ||
| total= totalPrice-(totalPrice*0.1); |
There was a problem hiding this comment.
Nitpick: You've created a new variable total, this is OK but you should always declare a new variable with let (or const if it's appropriate)
| function printOddNumbers(limit) {} | ||
| function printOddNumbers(limit) { | ||
| let count = 1; | ||
| while(count<=limit){ |
There was a problem hiding this comment.
Nitpick: The logic is correct, but the indentation seems incorrect here.
| @@ -45,7 +83,13 @@ function canRegister(age) {} | |||
| ) | |||
| */ | |||
|
|
|||
| function countReverse(number) {} | |||
| function countReverse(number) { | |||
| while( number >= 1){ | |||
There was a problem hiding this comment.
Suggestion: create a new variable that you can loop over and decrement rather than using the input number. e.g.
let currentNumber = number;
while (currentNumber >= minNum) {
console.log(number);
currentNumber--;
}
The reason for this is you still have access to the unmodified user input in case you need to use it in a future modification of the function.
Also you'll find in the future lessons that for certain types passed to a function modifying the input data could have an impact on other parts of the program (this isn't a problem for integers though so wouldn't be an issue here)
| else if (input == 0) | ||
| return 1; | ||
| else { | ||
| return (input * factorial(input - 1)); |
There was a problem hiding this comment.
Issue: This works, but isn't implemented using a loop as requested in the description. This uses a recursive function, which is an advanced topic we haven't covered yet.
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