Lotto Number Generator - Javascript

Question 1

Newbie to Javascript here and just practicing by making a random Lottery number generator. I am practicing by building simple, achievable projects.

The aim of this project is to make 10 lines of 6 numbers. Each line of numbers will take one random number from a 'hotNumbers' array, one random number from a 'luckyNumbers' array and 4 totally random numbers between 1-45.

I have it all working fine. The code just looks like it is repeating itself though, and I just wonder if there is a more efficient way to do what I'm doing here.

I've sat looking at it for hours, but I can't see a way to do things better, although I'm sure there is one. Is there a more effective and less convoluted way of displaying these values to HTML?

let hotNumbers = [34, 45, 13, 18, 38, 14, 43, 33, 20, 27, 17];
let luckyNumbers = [7, 11, 5, 31, 12, 30];
let newNumbers = [];
const line1 = document.querySelector('.numbers1')
const line2 = document.querySelector('.numbers2')
const line3 = document.querySelector('.numbers3')
const line4 = document.querySelector('.numbers4')
const line5 = document.querySelector('.numbers5')
const line6 = document.querySelector('.numbers6')
const line7 = document.querySelector('.numbers7')
const line8 = document.querySelector('.numbers8')
const line9 = document.querySelector('.numbers9')
const line10 = document.querySelector('.numbers10')
const generate = document.querySelector('.numbersButton');
const predictor = () => {
 reset();
 let randomHotNumber = hotNumbers[Math.floor(Math.random() * hotNumbers.length)];
 let randomLuckyNumber = luckyNumbers[Math.floor(Math.random() * luckyNumbers.length)];
 newNumbers.push(randomHotNumber, randomLuckyNumber);
 while (newNumbers.length < 6) {
 let newNumber = Math.floor(Math.random() *46);
 if (newNumbers.indexOf(newNumber) === -1 && newNumber != 0) {
 newNumbers.push(newNumber);
 }
 }
 newNumbers.sort(function(a, b){return a-b});
}
const reset = () => {
 newNumbers = [];
}
const lineNumbers = () => {
 predictor();
 line1.innerText = newNumbers;
 predictor();
 line2.innerText = newNumbers;
 predictor();
 line3.innerText = newNumbers;
 predictor();
 line4.innerText = newNumbers;
 predictor();
 line5.innerText = newNumbers;
 predictor();
 line6.innerText = newNumbers;
 predictor();
 line7.innerText = newNumbers;
 predictor();
 line8.innerText = newNumbers;
 predictor();
 line9.innerText = newNumbers;
 predictor();
 line10.innerText = newNumbers;
}
generate.addEventListener("click", lineNumbers);

<body>
<section>
 <h1>Lotto Numbers</h1>
</section>
<section class="numbers">
 <h2 class="numbers1"></h2>
 <h2 class="numbers2"></h2>
 <h2 class="numbers3"></h2>
 <h2 class="numbers4"></h2>
 <h2 class="numbers5"></h2>
 <h2 class="numbers6"></h2>
 <h2 class="numbers7"></h2>
 <h2 class="numbers8"></h2>
 <h2 class="numbers9"></h2>
 <h2 class="numbers10"></h2>
</section>
<section>
 <button class="numbersButton">New Numbers</button>
</section>
<script src="index.js"></script>
</body>

Question 2

First of all, I think you should get rid of reset and global newNumbers: that variable should be declared in predictor and returned from it, then predictor will become a pure function easy to test and follow. You could gather line1, line2, ..., line10 into an array and then for (const line of lines) line.innerText = predictor() - that will get rid of the repetition that (rightfully!) bothers you. Just a quick note for now, I'll try to leave a more comprehensive review tomorrow.

Question 3

Look at Rosetta code for Sattolo Cycle.

Question 4

When you find yourself doing thing1, thing2, thing3, ... thing452, the correct approach is an array and a loop:

const randInt = (lo, hi) =>
 Math.floor(Math.random() * (hi - lo)) + lo;
const sample = a => a[randInt(0, a.length)];
const generateNumbers = () => {
 const nums = [sample(hotNumbers), sample(luckyNumbers)];
 while (nums.length < cols) {
 const n = randInt(lowNumBound, highNumBound);
 if (!nums.includes(n)) {
 nums.push(n);
 }
 }
 return nums.sort((a, b) => a - b);
};
const handleNewNumbers = () => {
 numbersEl.innerHTML = [...Array(rows)]
 .map(() => `<h2>${generateNumbers().join(",")}</h2>`)
 .join("\n");
};
const hotNumbers = Object.freeze([
 34, 45, 13, 18, 38, 14, 43, 33, 20, 27, 17,
]);
const luckyNumbers = Object.freeze([7, 11, 5, 31, 12, 30]);
const rows = 10;
const cols = 6;
const lowNumBound = 1;
const highNumBound = 46;
const numbersEl = document.querySelector(".numbers");
const newNumbersBtn = document.querySelector(".new-numbers");
newNumbersBtn.addEventListener("click", handleNewNumbers);

<body>
 <section>
 <h1>Lotto Numbers</h1>
 </section>
 <section class="numbers"></section>
 <section>
 <button class="new-numbers">New Numbers</button>
 </section>
</body>

The [...Array(n)].map idiom might be confusing if you haven't seen it before. Array(n) makes an array of n items, [...] spreads the elements into a usable array, and .map is shorthand for this common loop operation that transforms each element, making a new array:

const a = [...Array(n)];
const result = [];
for (const _ of a) {
 result.push(`<h2>${generateNumbers().join(",")}</h2>`);
}
numbersEl.innerHTML = result.join("\n");

In this case, we don't care about the element, so it's ignored in the map callback.

Remarks about your approach:

Format code with Prettier.
Use kebab-case for CSS classes and ids.
Prefer arrow functions for callbacks (I use them for almost everything, except cases where I need this unbound).
Factor out repeated Math.random() logic.
Move constants to module- or class-level variables.
Use const, not let, unless you really must reassign something.
Scope data (except for those module-level constants) as tightly as possible (move newNumbers into predictor).
Name function as verbs (actions), not nouns--predictor should be predict, or something like that (but it looks like a number generator to me). One exception: you can omit the word "get" from the beginning of a function. So randInt() is OK even though it's a noun--getRandInt() is implied. getPredictor() doesn't seem right, but maybe it fits your Lotto domain I'm not familiar with.
Use Object.freeze() to make constant arrays immutable so you don't accidentally modify them.
Use !array.includes(item) rather than array.indexOf(item) === -1 to check presence.
Try to make functions idempotent (not modify global state). Prefer return values most of the time. This makes the function easy to test and keeps code loosely coupled, so changing a variable name elsewhere doesn't cause random functions to break. I've broken the rule a bit by having functions read from module-scoped constant state, but that's the point of the module--providing a namespace to "cheat" just enough for convenience. Parameters should be passed in if they're dynamic or specified by clients of the module. (I keep saying "module", but this is just a single file, so basically there's one global module for now).
Always use === and !==, never == and !=, which have surprising type coercion behavior that can cause subtle bugs.
Consider adding unit tests (you'll need to mock Math.random() so the code is deterministic).

Question 5

Seems a bit silly to call [...Array(n)].map confusing and then write something even more confusing..

Question 6

@JaDa How is that silly? I don't think [...Array(n)].map is confusing or I wouldn't have recommended it, just potentially confusing for newcomers to JS who aren't familiar with the Array constructor, spreading and mapping, as likely applies to OP. How is "the something even more confusing" confusing, exactly? It's just a loop pushing things onto an array explicitly, doing a map by hand--a common operation done all the time in programs. What do you suggest as the better way to write this loop?

Question 7

It was a little confusing at first. But after looking into it and trying it out the last couple of days it makes much more sense. Explanation was clear and to the point which helped a lot. Everything is difficult to first timers like me but you can only learn by doing the difficult stuff.

Question 8

`id` not `class`

Use element id to uniquely identify elements, not class.

<h2 class="numbers1"></h2>

becomes

<h2 id="numbers1"></h2>

Picking from a random set

You method of vetting you have selected a unique random item is flawed. It is possible that the second or more randomly selected number is never added the array because it has already been selected. Though the chance is very small it still exists.

You can use a random pick function. This removes the picked item so that it is never picked twice.

const items = [0, 1, 2, 3, 4, 5];
const rndPick = arr => arr.splice(Math.random() * arr.length | 0, 1)[0];
while (items.length > 0) { console.log(rndPick(items)); }

However this can be slower than the method you have used as each time an item is removed all the items above must be moved down one. The cost is about \$O(logn)\$

You can improve by swapping and keeping a track of number of picked items. (A modification of the shuffle algorithm)

const items = Object.assign([0,1,2,3,4,5], {size: 6});
const rndPick = arr => {
 const idx = Math.random() * (arr.size--) | 0;
 [arr[idx], arr[arr.size]] = [arr[arr.size], arr[idx]]; // swap
 return arr[arr.size]; 
}
while (items.size > 0) { console.log(rndPick(items)); }

The above random pick has a complexity of \$O(1)\$ and will never pick the same item twice. The set can be reset by setting items.size = items.length as it does not matter what order the items are to get a random set.

ggorlen ggorlenggorlen 4,1572 gold badges19 silver badges28 bronze badges · Accepted Answer · 2024-06-11 01:17:14Z

When you find yourself doing thing1, thing2, thing3, ... thing452, the correct approach is an array and a loop:

const randInt = (lo, hi) =>
 Math.floor(Math.random() * (hi - lo)) + lo;
const sample = a => a[randInt(0, a.length)];
const generateNumbers = () => {
 const nums = [sample(hotNumbers), sample(luckyNumbers)];
 while (nums.length < cols) {
 const n = randInt(lowNumBound, highNumBound);
 if (!nums.includes(n)) {
 nums.push(n);
 }
 }
 return nums.sort((a, b) => a - b);
};
const handleNewNumbers = () => {
 numbersEl.innerHTML = [...Array(rows)]
 .map(() => `<h2>${generateNumbers().join(",")}</h2>`)
 .join("\n");
};
const hotNumbers = Object.freeze([
 34, 45, 13, 18, 38, 14, 43, 33, 20, 27, 17,
]);
const luckyNumbers = Object.freeze([7, 11, 5, 31, 12, 30]);
const rows = 10;
const cols = 6;
const lowNumBound = 1;
const highNumBound = 46;
const numbersEl = document.querySelector(".numbers");
const newNumbersBtn = document.querySelector(".new-numbers");
newNumbersBtn.addEventListener("click", handleNewNumbers);

<body>
 <section>
 <h1>Lotto Numbers</h1>
 </section>
 <section class="numbers"></section>
 <section>
 <button class="new-numbers">New Numbers</button>
 </section>
</body>

The [...Array(n)].map idiom might be confusing if you haven't seen it before. Array(n) makes an array of n items, [...] spreads the elements into a usable array, and .map is shorthand for this common loop operation that transforms each element, making a new array:

const a = [...Array(n)];
const result = [];
for (const _ of a) {
 result.push(`<h2>${generateNumbers().join(",")}</h2>`);
}
numbersEl.innerHTML = result.join("\n");

In this case, we don't care about the element, so it's ignored in the map callback.

Remarks about your approach:

Format code with Prettier.
Use kebab-case for CSS classes and ids.
Prefer arrow functions for callbacks (I use them for almost everything, except cases where I need this unbound).
Factor out repeated Math.random() logic.
Move constants to module- or class-level variables.
Use const, not let, unless you really must reassign something.
Scope data (except for those module-level constants) as tightly as possible (move newNumbers into predictor).
Name function as verbs (actions), not nouns--predictor should be predict, or something like that (but it looks like a number generator to me). One exception: you can omit the word "get" from the beginning of a function. So randInt() is OK even though it's a noun--getRandInt() is implied. getPredictor() doesn't seem right, but maybe it fits your Lotto domain I'm not familiar with.
Use Object.freeze() to make constant arrays immutable so you don't accidentally modify them.
Use !array.includes(item) rather than array.indexOf(item) === -1 to check presence.
Try to make functions idempotent (not modify global state). Prefer return values most of the time. This makes the function easy to test and keeps code loosely coupled, so changing a variable name elsewhere doesn't cause random functions to break. I've broken the rule a bit by having functions read from module-scoped constant state, but that's the point of the module--providing a namespace to "cheat" just enough for convenience. Parameters should be passed in if they're dynamic or specified by clients of the module. (I keep saying "module", but this is just a single file, so basically there's one global module for now).
Always use === and !==, never == and !=, which have surprising type coercion behavior that can cause subtle bugs.
Consider adding unit tests (you'll need to mock Math.random() so the code is deterministic).

Seems a bit silly to call [...Array(n)].map confusing and then write something even more confusing..
@JaDa How is that silly? I don't think [...Array(n)].map is confusing or I wouldn't have recommended it, just potentially confusing for newcomers to JS who aren't familiar with the Array constructor, spreading and mapping, as likely applies to OP. How is "the something even more confusing" confusing, exactly? It's just a loop pushing things onto an array explicitly, doing a map by hand--a common operation done all the time in programs. What do you suggest as the better way to write this loop?
It was a little confusing at first. But after looking into it and trying it out the last couple of days it makes much more sense. Explanation was clear and to the point which helped a lot. Everything is difficult to first timers like me but you can only learn by doing the difficult stuff.

Stack Exchange Network

Lotto Number Generator - Javascript

2 Answers 2

`id` not `class`

Picking from a random set

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Lotto Number Generator - Javascript

2 Answers 2

id not class

Picking from a random set

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions

`id` not `class`