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

finished task #218

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

finished task #218

wants to merge 3 commits into from

Conversation

artkas03
Copy link

No description provided.

Copy link

@And678 And678 left a comment

Choose a reason for hiding this comment

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

Input should not contain duplicate digits as well, show error when it has duplicates. Same for having letters in input:
image

Comment on lines 3 to 4
const targetNumber = Math.round(Math.random() * 9000 + 999).toString();

Copy link

Choose a reason for hiding this comment

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

This will generate duplicate digits:
image


const targetNumber = Math.round(Math.random() * 9000 + 999).toString();

module.exports = { targetNumber };
Copy link

Choose a reason for hiding this comment

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

Better export a function to generate a number, not the number itself, as per requirements:

Please create separate modules for generating a number...

src/terminal.js Outdated
Comment on lines 9 to 13
return new Promise((resolve) => {
terminal.question('Please enter your guess numeber: ', (number) => {
return resolve(number);
});
});
Copy link

Choose a reason for hiding this comment

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

This is 🔥

src/app.js Outdated
const [bulls, cows] = checkBullsAndCows(targetNumber, userNumber);

terminal.write(
`\nNumber of bulls - ${bulls}, number of cows - ${cows} ${targetNumber}\n`
Copy link

Choose a reason for hiding this comment

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

Not fun this way. ;)

Suggested change
`\nNumber of bulls - ${bulls}, number of cows - ${cows} ${targetNumber}\n`
`\nNumber of bulls - ${bulls}, number of cows - ${cows}\n`

Copy link

@And678 And678 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, approve ✅

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.

2 participants