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

Solution #419

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 60 additions & 1 deletion src/app.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,62 @@
'use strict';
/* eslint-disable no-console */

// Write your code here
const readline = require('readline');
const { checkIsValidUserInput } = require('./modules/checkIsValidUserInput');
const { generateRandomNumber } = require('./modules/generateRandomNumber');
const { getBullsAndCows } = require('./modules/getBullsAndCows');

const terminal = readline.createInterface(process.stdin, process.stdout);

const guessNumber = generateRandomNumber();
let count = 0;

function playGame(question = '') {
terminal.question(question, (userInput) => {
if (!checkIsValidUserInput(userInput)) {

Choose a reason for hiding this comment

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

Code quality issue: It's a good practice to keep your functions small and focused on doing one thing. The playGame function is doing multiple things: validating the user input, incrementing the count, getting the result, logging the result, checking the result, and deciding what to do next. Consider breaking it down into smaller functions to improve readability and maintainability.

console.log(
'It is not valid number! Input the 4 dight number, without duplicates!',
);
playGame('Try again!');

Choose a reason for hiding this comment

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

Bug: The 'Try again!' message is not very informative. It would be better to tell the user exactly what they did wrong and how they can correct it.

} else {
count++;

const result = getBullsAndCows(userInput, guessNumber);

console.log(result, '(Try', count, ')');

Choose a reason for hiding this comment

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

Code quality issue: The way you are logging the result is a bit confusing. It would be better to use a template string to make it clearer. For example: console.log(${result} (Try ${count}));


if (result.bulls === 4) {
console.log(
'Congratulation!!! You win!!! Number is... ',

Choose a reason for hiding this comment

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

Code quality issue: Consider using a template string for the win message as well. For example: console.log(Congratulation!!! You win!!! Number is... ${guessNumber} [From ${count} try!]);

guessNumber,
` [From ${count} try!]`,
);
terminal.close();
} else {
const { bulls, cows } = result;

if (bulls === 0 && cows === 0) {

Choose a reason for hiding this comment

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

Recommendation: The conditions on lines 38, 42, and 46 are a bit confusing. It would be clearer to use a switch statement or an if-else-if chain instead. This would also allow you to remove the redundant conditions on lines 47-51.

playGame('Dont worry, it happens...');
}

if (bulls === 1 || cows === 1) {
playGame('You are on the right track!');
}

if (
bulls === 2 ||
bulls === 3 ||
cows === 2 ||
cows === 3 ||
cows === 4
) {
playGame('So close... Come on!');
}

Choose a reason for hiding this comment

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

Code quality issue: The message 'So close... Come on!' is not very informative. It would be better to tell the user exactly what they did right and how they can improve.

}
}
});
}

console.log('Lets play the game "Bulls and Cows"!!!');

Choose a reason for hiding this comment

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

Code quality issue: The console.log statements on lines 60 and 61 could be combined into one for better readability. For example: console.log('Lets play the game "Bulls and Cows"!!! Input the 4 digit number');

console.log('Input the 4 dight number');
playGame();
13 changes: 12 additions & 1 deletion src/modules/checkIsValidUserInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,18 @@
* @return {boolean} - True if the user input is valid, false otherwise
*/
function checkIsValidUserInput(userInput) {
/* Write your code here */
const is4digitNumber = userInput.length === 4;
const isAllAreNumbers = userInput.split('').every((x) => /^[0-9]$/.test(x));
const isNotStartZero = userInput[0] !== '0';
const isNotDuplicates = userInput
.split('')
.every((x, _, arr) => arr.indexOf(x) === arr.lastIndexOf(x));

if (is4digitNumber && isAllAreNumbers && isNotStartZero && isNotDuplicates) {
return true;
} else {
return false;
}
Comment on lines +19 to +23

Choose a reason for hiding this comment

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

This if-else statement is unnecessary. You can directly return the result of the condition in the if statement. It will automatically be converted to a boolean value. This will make your code more concise and readable.

}

module.exports = {
Expand Down
13 changes: 13 additions & 0 deletions src/modules/generateRandomNumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@
*/
function generateRandomNumber() {
/* Write your code here */
const availableNums = [1, 2, 3, 4, 5, 6, 7, 8, 9];
const result = [];

const getRandomN = () => Math.floor(Math.random() * availableNums.length);

Choose a reason for hiding this comment

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

It's better to use more descriptive function names. getRandomN doesn't give a clear understanding of what this function does. Consider renaming it to something like getRandomIndex.


result.push(availableNums.splice(getRandomN(), 1)[0]);

Choose a reason for hiding this comment

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

The use of splice method here can be confusing. It's better to avoid changing the original array. Consider using a different approach to select a random element from the array without modifying it.

availableNums.unshift(0);

Choose a reason for hiding this comment

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

The use of unshift here is not clear. If you need to add 0 to the list of available numbers, why not include it in the original array? Also, it's not clear why 0 is added after the first number is selected. This could lead to confusion for other developers reading your code.


while (result.length < 4) {
result.push(availableNums.splice(getRandomN(), 1)[0]);
}

return Number(result.join(''));

Choose a reason for hiding this comment

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

Converting an array of numbers to a string and then back to a number can be unnecessary and may lead to confusion. Consider returning the array itself or joining the array elements into a string and returning that string.

}

module.exports = {
Expand Down
17 changes: 16 additions & 1 deletion src/modules/getBullsAndCows.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,22 @@
* Example: { bulls: 1, cows: 2 }
*/
function getBullsAndCows(userInput, numberToGuess) {
/* Write your code here */
const userInputArr = userInput.toString().split('');
const numberToGuessArr = numberToGuess.toString().split('');
const result = {
bulls: 0,
cows: 0,
};

userInputArr.forEach((el, i) => {
if (el === numberToGuessArr[i]) {
result.bulls++;
} else if (numberToGuessArr.includes(el)) {
result.cows++;
}
});
Comment on lines +23 to +29

Choose a reason for hiding this comment

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

Consider using a different strategy to improve the performance of this code. For instance, you could create a map (an object in JavaScript) to store the count of each digit in numberToGuessArr. Then, for each digit in userInputArr, you can check if it's in the map and update the counts of bulls and cows accordingly. This will reduce the time complexity to linear.


return result;
}

module.exports = {
Expand Down
Loading