Skip to content
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

Created bulls and cows game #244

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

davinnchii
Copy link

No description provided.

Copy link

@voievodik voievodik left a comment

Choose a reason for hiding this comment

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

Good job!
But your random number is generated every time you call the function. I think the main idea of the game is to guess a stable random number. (You just need to subtract the random number from the function)

Copy link

@ArtemTeslenko ArtemTeslenko left a comment

Choose a reason for hiding this comment

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

Good job! Do not change the source number on each user attempt.

src/app.js Outdated
return;
}

const searchedNumber = generateNumber();

Choose a reason for hiding this comment

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

if user did not win, this number will be generated again and overwrite? In this case it is possible to win only by guessing all the numbers at once.

Copy link

@DenisChakhalian DenisChakhalian left a comment

Choose a reason for hiding this comment

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

good job!

src/app.js Outdated

function gameStart() {
terminal.question('Enter a number: ', (userInput) => {
if (userInput.length !== 4 || !Number(userInput)) {

Choose a reason for hiding this comment

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

4 is a magic value

Choose a reason for hiding this comment

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

Not fixed

src/app.js Outdated
terminal.question('Enter a number: ', (userInput) => {
if (userInput.length !== 4 || !Number(userInput)) {
console.log('Invalid number, please enter a 4 different digits');
terminal.question('Enter a number: ', gameStart);

Choose a reason for hiding this comment

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

Suggested change
terminal.question('Enter a number: ', gameStart);
gameStart();


const userNumberArr = userNumber.split('');

for (let i = 0; i < 4; i++) {

Choose a reason for hiding this comment

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

4 is a magic value

Comment on lines 10 to 11
if (searchedNumber.includes(userNumberArr[i])) {
if (searchedNumber.indexOf(userNumberArr[i]) === i) {

Choose a reason for hiding this comment

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

Try not to use nested if

}
}

return [bulls, cows];

Choose a reason for hiding this comment

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

Better to return object
So when you do destructuring order doesn't matter

'use strict';

function generateNumber() {
return Math.floor(1000 + Math.random() * 9000).toString();

Choose a reason for hiding this comment

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

Magic values

Copy link

@Moroz-Dmytro Moroz-Dmytro left a comment

Choose a reason for hiding this comment

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

Small question below

src/app.js Outdated

function gameStart() {
terminal.question('Enter a number: ', (userInput) => {
if (userInput.length !== 4 || !Number(userInput)) {

Choose a reason for hiding this comment

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

Not fixed

src/app.js Outdated
return;
}

const [bulls, cows] = checkAnswer(searchedNumber, userInput);

Choose a reason for hiding this comment

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

Does it work correctly now? You return object, not array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants