-
-
Notifications
You must be signed in to change notification settings - Fork 438
Glasgow class 6- Yesna Omar- JavaScript- week 2 #448
base: main
Are you sure you want to change the base?
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.
Very nice work. Maybe you can take some time this Saturday to work on my comments?
let isBigEnough; | ||
|
||
if (isBigEnough) { | ||
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.
Can you tell me what you think this line does?
@@ -4,7 +4,14 @@ | |||
1. the user should be 18 or older | |||
2. the user must be logged in | |||
*/ | |||
function isAcceptableUser(userAge, isLoggedIn) {} | |||
function isAcceptableUser(userAge, isLoggedIn) { | |||
userAge == 18; |
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.
Again, you have this same coding pattern. What does it do?
userAge == 18; | ||
if(userAge >= 18 && isLoggedIn){ | ||
return true; | ||
}else{ |
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 seem to not have format on save turned on (or it is conflicting with your auto-save?). Could you work with your buddy or a volunteer on Saturday to fix this?
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.
I am experiencing the same problem even though my format on save option is turned on. I will take a look at it on Saturday to see if I can fix it with volunteers help
function applyDiscount(totalPrice) {} | ||
function applyDiscount(totalPrice, discount) { | ||
if (totalPrice >200) { | ||
total= totalPrice-(totalPrice*0.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.
total should be defined with let
or const
total= totalPrice-(totalPrice*0.1); | ||
return total; | ||
} else if (totalPrice < 200){ | ||
total=totalPrice-(totalPrice*0.05); |
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.
These two lines occur twice. Can you see how if you had a new user requirement (for example, to add some rounding), you would need to change your code in 2 places. Could you rewrite this code to make it less verbose?
function canRegister(age) { | ||
if (age <= 12) { | ||
return "You Are Too Young To Register" | ||
} else if(age > 12 && age <90){ |
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 to write age > 12 in this line? What would happen if you remove it?
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