Skip to content

HannahN#15

Open
hannahwn wants to merge 12 commits intoHackYourAssignment:mainfrom
hannahwn:main
Open

HannahN#15
hannahwn wants to merge 12 commits intoHackYourAssignment:mainfrom
hannahwn:main

Conversation

@hannahwn
Copy link

submitting assingment

@mo92othman mo92othman self-assigned this Jan 24, 2026
Copy link

@mo92othman mo92othman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hannahwn ! Thanks for submitting the assignment! Nice job on this assignment! Well done!

I’ve left a few comments. Let me know if anything is unclear.

Comment on lines +10 to +12
let UserInput = prompt('Enter an year: ');

let year = Number(UserInput);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clear variable names. Since these variables are not reassigned, you could use const instead of let to make that clearer. This is a small best-practice improvement.

Comment on lines +21 to +24
else if(year % 100 ===0 || !year % 4 ===0){

console.log('no,' +year+' is not a leap year');
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a small mistake in this part. !year % 4 === 0 does not mean “year is not divisible by 4”.

year is a number → usually truthy (Why is it a number? check line 12 in your code)
!year becomes false
false % 4 becomes 0
0 === 0 is true
Example: with input 2024, your code can end up printing “not a leap year”, but 2024 is a leap year.
Could you think of a way to fix this?


One more thing in this part, since that detials matters. console.log('no,' +year+' is not a leap year');
The n in no should be Upper case.

let year = Number(UserInput);


if(year<1 || year >9999 || isNaN(year)){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job using isNaN(year) to handle invalid input 👍

Comment on lines +16 to +24
console.log('Invalid Year');}

else if(year % 400 === 0 ){
console.log('Yes,' +year+' is a leap year');
}
else if(year % 100 ===0 || !year % 4 ===0){

console.log('no,' +year+' is not a leap year');
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small style note: consistent indentation (spacing at the beginning of lines) makes the code easier to read. Try to align if / else blocks consistently.

Suggested change
console.log('Invalid Year');}
else if(year % 400 === 0 ){
console.log('Yes,' +year+' is a leap year');
}
else if(year % 100 ===0 || !year % 4 ===0){
console.log('no,' +year+' is not a leap year');
}
console.log('Invalid Year');
} else if (year % 400 === 0) {
console.log('Yes, ' + year + ' is a leap year');
} else if (year % 100 === 0 || !year % 4 === 0) {
console.log('No, ' + year + ' is not a leap year');
}

}


if (foundCorrectUser === true) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small style note, what you wrote is correct. But you can also write it in another way:
since foundCorrectUser is already a boolean, you can simply write if (foundCorrectUser) {. Both are correct.

const eurAmountInput = prompt("Enter amount in EUR: ");
const eurAmountNum = Number(eurAmountInput);
if (Number.isNaN(eurAmountNum) || eurAmountNum > 0) {
if (Number.isNaN(eurAmountNum) || eurAmountNum < 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can also reject 0. Using <= 0 instead of < 0 would handle this edge case.

} else {
const eurAmount = usdAmountNum / eur_usd_rate;
const eurAmount = usdAmountNum / EUR_USD_RATE;
console.log(usdAmountNum.toFixed(2) + ' USD is equal to ' + usdAmountNum.toFixed(2) + ' EUR.');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are printing the USD amount twice. You should display the calculated EUR amount instead.

} else {
}
else if (menuSelection === "3"){
console.log("The current exchange rate is 1 EUR = 1.1643 USD.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small improvement: instead of hardcoding the exchange rate value, you could use the EUR_USD_RATE constant here. This keeps the output consistent and makes the code easier to maintain if the rate changes later for example.

console.log("The current exchange rate is 1 EUR = 1.1643 USD.");
}
else {
console.log("Invalid selection. Please choose either 1 or 2.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here in line 41. The menu has three options. The invalid selection message should also mention options 1, 2, or 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants