Diana C. #4
Diana C. #4
Conversation
mnvacym
left a comment
mnvacym
left a comment
There was a problem hiding this comment.
Good job on your assignment! I have left some comments for the improvements that need to be implemented. Let me know if anything is not clear.
There was a problem hiding this comment.
I think you ran the command npm install prompt-sync on the root level. You don't need package.json on root level. You already have one inside the task-1 folder. You can remove package.json and package-lock.json from the root level.
| // Step 3: Implement the logic | ||
|
|
||
|
|
||
| const year = Number(prompt('Enter to check the year: ')); |
There was a problem hiding this comment.
Good that you converted the user input to a Number type. But it is a good practice to create readable variables with clear names so that future developers can understand the code more easily. Like, for example:
const userInput = prompt('Enter to check the year: ');
const year = Number(userInput);
| } else if ((year % 100) === 0) { | ||
| console.log(`No, ${year} is not a leap year`); | ||
| } else if ((year % 4) === 0) { | ||
| console.log(`Yes, ${year} is a leap year`); |
There was a problem hiding this comment.
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, in this case, for example, you could create a variable called isLeapYear and reuse it wherever needed. Also, this way you could avoid repeating the same console statement, and 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`);
}
|
|
||
| // Write your code here. | ||
| // Use the variables 'username' and 'password' to access the input values | ||
| // Use incorrectAttempts to track the number of failed attempts |
There was a problem hiding this comment.
No need to move these comments. Either you can remove them or you can keep them in the same place.
| // Write your code here. | ||
| // Use the variables 'username' and 'password' to access the input values | ||
| // Use incorrectAttempts to track the number of failed attempts | ||
| if ((username === "admin" && password === "Hack1234") || (username === "user" && password === "7654321")) { |
There was a problem hiding this comment.
The first thing you should check in your logic is that if incorrectAttempts is 4 or more, you need to show the correct message accordingly. In your current logic, even if the incorrect attempts exceed 3, if the user enters correct credentials, it will still show the success message.
| console.log("Hello and welcome to the currency converter. Please choose: ") | ||
| console.log("1: Convert EUR to USD") | ||
| console.log("2: Convert USD to EUR") | ||
| console.log("3: Display the current exchange rate") |
There was a problem hiding this comment.
You are not supposed to remove the semicolons at the end of the lines.
| 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.
Based on the error message, the logic should check if the inputted amount is a valid positive number:
eurAmountNum <= 0
| // USD to EUR | ||
| const usdAmountInput = prompt("Enter amount in USD: "); | ||
| const usdAmountNum = Number(usdAmountInput); | ||
| if (Number.isNaN(usdAmountNum) || usdAmountNum < 0) { |
There was a problem hiding this comment.
Based on the error message, the logic should check if the inputted amount is a valid positive number:
usdAmountNum <= 0
| } 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.'); |
There was a problem hiding this comment.
You should use the correct variable.
eurAmount.toFixed(2)
| console.log("Invalid selection. Please choose either 1 or 2."); | ||
| } else if (menuSelection === "3"){ | ||
| //The current exchange rate | ||
| console.log ("The current exchange rate is 1 EUR = 1.1643 USD") |
There was a problem hiding this comment.
The output is correct. However, it is a good practice if you reuse the existing variable:
console.log(`The current exchange rate is 1 EUR = ${EUR_USD_RATE} USD.`);
2 participants
No description provided.