-
Notifications
You must be signed in to change notification settings - Fork 19
Mareh A. #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Mareh A. #22
Conversation
mnvacym
left a comment
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.
Good job on your assignment! I have left some comments that need to be implemented. Let me know if anything is unclear.
|
|
||
| //Task 1 Mareh | ||
| let userInput = prompt('Enter the year: '); | ||
| let year = Number(userInput); |
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 that you used variables with clear names. But since these are constant variables, meaning that they are not re-assigned a value, it is a good practice to use const instead of let. So that when a future developer reads this code, it is clear that these variables are constant and don't change.
|
|
||
| if (isNaN(year) || year < 1 || year > 9999) { | ||
| console.log('Invalid year!'); | ||
| } else if ((year % 400 === 0) || (year % 4 === 0 && year % 100 !== 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 that you met all the required conditions. However, one improvement would be to follow the DRY principle. In programming, there is a principle called DRY (Don't Repeat Yourself). So, for example, you could create a variable called isLeapYear and reuse it. Also, this way, the code becomes more readable for the future. Like, for example:
const isLeapYear = (year % 4 === 0) && (year % 100 !== 0 || year % 400 === 0);
if (isLeapYear) {
console.log(`Yes, ${year} is a leap year`);
} else {
console.log(`No, ${year} is not a leap year`);
}
| successMessage('Logged in successfully.'); | ||
| } | ||
| else { | ||
| incorrectAttempts+=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.
At this point, you need to check if incorrectAttempts has reached 4. If it is 4, you need to show the error message.
if (incorrectAttempts === 4) {
errorMessage('Login blocked: Too many incorrect attempts');
return;
}
| const eurAmountInput = prompt("Enter amount in EUR: "); | ||
| const eurAmountNum = Number(eurAmountInput); | ||
| if (Number.isNaN(eurAmountNum) || eurAmountNum > 0) { | ||
| if (Number.isNaN(eurAmountNum) && eurAmountNum > 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.
Your logic for checking if the inputted EUR amount is incorrect. You should check if the amount is NOT a positive number:
eurAmountNum <= 0
| const usdAmountInput = prompt("Enter amount in USD: "); | ||
| const usdAmountNum = Number(usdAmountInput); | ||
| if (Number.isNaN(usdAmountNum) || usdAmountNum < 0) { | ||
| if (Number.isNaN(usdAmountNum) && usdAmountNum > 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.
Based on the error message, the logic should check if the inputted amount is a valid positive number:
usdAmountNum <= 0
| } else if(menuSelection === "3") { | ||
| console.log("The current exchange rate is 1 EUR = " + EUR_USD_RATE + " USD."); | ||
| }else { | ||
| console.log("Invalid selection. Please choose either 1 or 2 ,or 3."); |
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 output is grammatically not correct. It should be: "Invalid selection. Please choose either 1, 2 or 3."
No description provided.