-
-
Notifications
You must be signed in to change notification settings - Fork 438
final change- week2 #451
base: main
Are you sure you want to change the base?
final change- week2 #451
Conversation
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.
Hey @AkramIzedi - Could you also update your Pull Request title to fit the format please?
I've left a couple of comments related to the code as well ^^,
overall, you're getting there!
|
||
if (isHappy) { | ||
function getMood(happy) { | ||
if (happy == true) { |
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.
Since booleans can only be true or false you shouldn't need to check if happy is equal to true or false. Checking the variable happy should be enough to determine whether the mood is happy or not.
You can read more on booleans here: https://www.w3schools.com/js/js_booleans.asp
return "I am happy"; | ||
} else { | ||
} else if (happy == false){ |
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.
since happy can only be either true or false and we want to make sure to enter this block and return "I am not happy" when happy is false, the if else statement should be enough to determine what to return and we shouldn't need a happy == false
check
|
||
if (isBigEnough) { | ||
if (num>10) { |
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.
Nice one!
function isAcceptableUser(userAge, isLoggedIn) { | ||
if (userAge>=18 && isLoggedIn===true){ | ||
return true; | ||
}else if (userAge>18 && isLoggedIn===true){ | ||
return false; | ||
}else { | ||
return false; | ||
} | ||
} |
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.
The condition to accept the user or not is determining whether userAge >= 18 and if the user is logged in
userAge >= 18
-> this statement should return a boolean, so if you assign it to a variable, that variable's type should be a boolean. You can see the example code to see this here:
let isUserAgeBigger = userAge >= 18;
console.log(typeof isUserAgeBigger); // this should be a boolean
console.log(isUserAgeBigger) // this should be the value of it
the variable isLoggedIn is already of type boolean, as you can see in the examples from the tests here
Since you only want a boolean return from this function, you could just evaluate the boolean expression, by returning the statement inside the first if
condition, since it will only return true if the statement inside the if condition (userAge>=18 && isLoggedIn===true
) is true.
So you could just do:
function isAcceptableUser(userAge, isLoggedIn) {
return userAge>=18 && isLoggedIn===true
}
and it should return the behaviour above is doing.
Also since isLoggedIn
is a boolean value, you do not need to check if it's equal to true or not. you should be able to directly use it in the conditions.
function applyDiscount(totalPrice) { | ||
if (totalPrice>200){ | ||
return totalPrice *0.9; | ||
}else if (totalPrice<200){ | ||
return totalPrice * 0.95; | ||
} | ||
} |
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.
Nice one! ☝️
This is exactly how the else if
statement should be used, because we have a third condition where the totalPrice
could be exactly equal to 200, but we haven't been requested to make a discount when the totalPrice is 200!
We should also return totalPrice
just to ensure we have all cases covered (even though i'm aware there are no tests for this 😅 feel free to add one and try it out ^^)
for (let i = 1; i <= limit; i++) { | ||
if (i % 2 === 1) { | ||
console.log(i); | ||
} | ||
} |
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.
Awesome 👌
if (price1 <= 0 || price2 <= 0) { | ||
return "Invalid prices. Prices must be more than 0."; | ||
} |
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.
Nice 😊
Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in
HOW_TO_MARK.md
in 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?