add task solution #423
add task solution #423
Conversation
There was a problem hiding this comment.
There are a few critical issues that need to be addressed: the logic for counting cows and bulls needs correction, and the number generation range is incorrect. Please fix these to ensure the program functions as intended. Keep up the good work, and don't hesitate to reach out if you need help! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/app.js
Outdated
|
|
||
| terminal.question('Guess the number', (userNumber) => { | ||
| if (!checkIsValidUserInput(userNumber)) { | ||
| return; |
There was a problem hiding this comment.
The program should provide feedback to the user if the input is invalid. Consider adding a message to inform the user that their input was not valid.
| } | ||
|
|
||
| if (userNumber === numberToGuess) { | ||
| terminal.close(); |
There was a problem hiding this comment.
After closing the terminal, it might be a good idea to inform the user that they have guessed the correct number. Consider adding a message to congratulate the user.
src/app.js
Outdated
| if (userNumber === numberToGuess) { | ||
| terminal.close(); | ||
| } else { | ||
| getBullsAndCows(userNumber, numberToGuess); |
There was a problem hiding this comment.
The result of getBullsAndCows is not being used. Consider storing the result and providing feedback to the user about the number of bulls and cows.
| for (const num of arr) { | ||
| if (arr.filter((number) => number === num).length > 1 || isNaN(+num)) { |
There was a problem hiding this comment.
The current logic checks for duplicate digits by filtering the array for each digit. This results in a time complexity of O(n^2), which is inefficient for this task. Consider using a Set to track seen digits, which will improve the efficiency of the duplicate check.
| const arr = [...userInput]; | ||
|
|
||
| for (const num of arr) { | ||
| if (arr.filter((number) => number === num).length > 1 || isNaN(+num)) { |
There was a problem hiding this comment.
The condition 'isNaN(+num)' is used to check if the character is a number, but this is unnecessary since the input is expected to be a string of digits. If you want to ensure the input consists only of digits, consider using a regular expression at the beginning of the function.
src/modules/generateRandomNumber.js
Outdated
| const numArr = [Math.floor(Math.random() * 9) + 1]; | ||
|
|
||
| while (numArr.length < 4) { | ||
| const newNum = Math.floor(Math.random() * 9); |
There was a problem hiding this comment.
The range for generating 'newNum' should be from 0 to 9, but the current implementation only generates numbers from 0 to 8. To include 9, the multiplication factor should be 10 instead of 9.
| } else if (guessArr.includes(userArr[i])) { | ||
| score.cows += 1; |
There was a problem hiding this comment.
There is a logical issue here. The current logic increments cows even if the digit has already been counted as a bull. You need to ensure that a digit is not counted as a cow if it has already been counted as a bull.
There was a problem hiding this comment.
Great job on your progress so far! 🚀 While there are some issues to address, such as ensuring cows aren't double-counted as bulls, adjusting the random number generation to include 9, and optimizing duplicate digit checks with a Set, these are excellent learning opportunities. Additionally, remember to display the bulls and cows count and provide feedback when the user guesses correctly, using console.log for messages. Keep up the good work and use these insights to refine your code. You're on the right track! 🌟
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/app.js
Outdated
|
|
||
| terminal.question('Guess the number', (userNumber) => { | ||
| if (!checkIsValidUserInput(userNumber)) { | ||
| return 'Input is not a valid 4-digit number'; |
There was a problem hiding this comment.
The return statement here will not send the message to the user. Consider using console.log to display the message to the user.
| return 'Input is not a valid 4-digit number'; | ||
| } | ||
|
|
||
| if (userNumber === numberToGuess) { |
There was a problem hiding this comment.
You should provide feedback to the user when they guess the number correctly, such as a congratulatory message.
src/app.js
Outdated
| if (userNumber === numberToGuess) { | ||
| terminal.close(); | ||
| } else { | ||
| getBullsAndCows(userNumber, numberToGuess); |
There was a problem hiding this comment.
The result of getBullsAndCows is not being used. You might want to display the bulls and cows count to the user.
| for (const num of arr) { | ||
| if (arr.filter((number) => number === num).length > 1 || isNaN(+num)) { |
There was a problem hiding this comment.
The current logic checks for duplicate digits by filtering the array for each digit and checking the length. This approach is inefficient. Consider using a Set to track seen digits, which would simplify the logic and improve performance.
src/modules/generateRandomNumber.js
Outdated
| const numArr = [Math.floor(Math.random() * 9) + 1]; | ||
|
|
||
| while (numArr.length < 4) { | ||
| const newNum = Math.floor(Math.random() * 9); |
There was a problem hiding this comment.
The range for generating newNum should be from 0 to 9 inclusive, but Math.floor(Math.random() * 9) generates numbers from 0 to 8. To include 9, you should use Math.floor(Math.random() * 10).
| } else if (guessArr.includes(userArr[i])) { | ||
| score.cows += 1; |
There was a problem hiding this comment.
The current logic for counting cows is incorrect because it counts a cow even if the digit has already been counted as a bull. You should ensure that a digit is not counted as a cow if it has already been counted as a bull.
No description provided.