-
Notifications
You must be signed in to change notification settings - Fork 535
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 #318
base: master
Are you sure you want to change the base?
Solution #318
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! Let's improve it a bit
src/modules/generateRandomNumber.js
Outdated
let randomNumber = ''; | ||
const previousNumbers = []; | ||
|
||
for (let i = 0; i < 4;) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you use increment here?
src/modules/generateRandomNumber.js
Outdated
let randomNumber = ''; | ||
const previousNumbers = []; | ||
|
||
for (let i = 0; i < 4;) { | ||
const randomNum = Math.floor(Math.random() * 10); | ||
|
||
if (randomNum === 0 && i === 0) { | ||
continue; | ||
} | ||
|
||
if (previousNumbers.includes(randomNum)) { | ||
continue; | ||
} | ||
|
||
previousNumbers.push(randomNum); | ||
randomNumber += randomNum.toString(); | ||
i++; | ||
} | ||
|
||
return +randomNumber; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let randomNumber = ''; | |
const previousNumbers = []; | |
for (let i = 0; i < 4;) { | |
const randomNum = Math.floor(Math.random() * 10); | |
if (randomNum === 0 && i === 0) { | |
continue; | |
} | |
if (previousNumbers.includes(randomNum)) { | |
continue; | |
} | |
previousNumbers.push(randomNum); | |
randomNumber += randomNum.toString(); | |
i++; | |
} | |
return +randomNumber; | |
let res = ''; | |
const numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; | |
for (let i = 0; i < 4; i++) { | |
const index = Math.floor(Math.random() * (10 - i)); | |
res += numbers.splice(index, 1)[0]; | |
} | |
return res; |
src/app.js
Outdated
if (checkIsValidUserInput(number)) { | ||
userInput = number; | ||
} else { | ||
terminal.write('Invalid input\n'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be great to specify what the user has done wrong. You can do it inside the validation function
src/app.js
Outdated
userInput = number; | ||
} else { | ||
terminal.write('Invalid input\n'); | ||
askForNumber().then(resolve); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to use additional resolve
here
Your code for generating number dont pass 2 tests, so i didnt use it. Improved validation function, now it gives certain information whats wrong. Also changed a bit asdForNumber function and i need both resolves, if i remove one of them the program will not work with wrong input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous mentor's comments were not fixed
What didnt fixed? I wrote a message, where explained all details, if u provide a comment u should describe what i need to do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check comments and fix the code.
src/app.js
Outdated
userInput = number; | ||
resolve(); | ||
} else { | ||
askForNumber().then(resolve); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should remove redundant then chain
src/modules/generateRandomNumber.js
Outdated
let randomNumber = ''; | ||
const previousNumbers = []; | ||
|
||
for (let i = 0; i < 4;) { | ||
const randomNum = Math.floor(Math.random() * 10); | ||
|
||
if (randomNum === 0 && i === 0) { | ||
continue; | ||
} | ||
|
||
if (previousNumbers.includes(randomNum)) { | ||
continue; | ||
} | ||
|
||
previousNumbers.push(randomNum); | ||
randomNumber += randomNum.toString(); | ||
i++; | ||
} | ||
|
||
return +randomNumber; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block can be modified in the next way
let randomNumber = ''; | |
const previousNumbers = []; | |
for (let i = 0; i < 4;) { | |
const randomNum = Math.floor(Math.random() * 10); | |
if (randomNum === 0 && i === 0) { | |
continue; | |
} | |
if (previousNumbers.includes(randomNum)) { | |
continue; | |
} | |
previousNumbers.push(randomNum); | |
randomNumber += randomNum.toString(); | |
i++; | |
} | |
return +randomNumber; | |
function generateRandomNumber() { | |
const secretCode = Array.from({ length: 4 }, (el) => 'x'); | |
for (let i = 0; i < secretCode.length; i++) { | |
secretCode[i] = assignRandomNumber(secretCode); | |
} | |
return Number(secretCode.join('')); | |
} | |
const assignRandomNumber = (secretCode) => { | |
const number = Math.floor(Math.random() * 9 + 1); | |
if (secretCode.includes(number)) { | |
return assignRandomNumber(secretCode); | |
} | |
return number; | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done
No description provided.