The onclick event and the function is working properly. The image didn’t update is because the randomNum is the same as the previous one.
Our goal is to prevent using the same number that is generated previously.
One way to do this is to keep track of the last generated random number, and regenerate until it is different from the previous one.
Main logic
let randomNum;
do {
randomNum = Math.floor(Math.random() * images.length);
} while (randomNum === prevRandomNum);
prevRandomNum = randomNum
Full code (With some modification of your original code)
// Keep track of the last random number
let prevRandomNum = -1
function my(){
const btn = document.getElementById("btng");
btn.innerText = "Suggest me another";
const images = ["im/R.jpg","im/S.jpg","im/E.jpg"];
let randomNum;
do {
randomNum = Math.floor(Math.random() * images.length);
} while (randomNum === prevRandomNum);
prevRandomNum = randomNum
const backgroundImage = images[randomNum];
document.getElementById("pic").style.backgroundImage=`url(${backgroundImage})`;
}
Solution 2 :
Please make the button type=”button”. it seems working.
<button id="btng" type="button" onclick="my()">Suggest me another</button>
Solution 3 :
As @AndrewLi already answered, there is possibilty that generated random number is the same as the last one.
Instead of generating new number multiple times (with while loop).
You can filter out currently displayed image from array.
function my(){
var bt=document.getElementById("btng");
bt.textContent = "Suggest me another";
var img = document.getElementById("img");
var my = new Array("/im/R.jpg","/im/S.jpg","/im/E.jpg").filter((src) => src !== new URL(img.src).pathname);
var randomNum = Math.floor(Math.random() * my.length);
var backgroundImage = my[randomNum];
document.getElementById("img").src = my[randomNum];
document.getElementById("pic").style.backgroundImage=`url(${backgroundImage})`;
}
But to make it work, you need provide absolute path to your images in array.
So /im/R.jpg instead of im/R.jpg
Solution 4 :
Here is the probably most efficient way to do this without filtering the values causing a loop or re-generating values in case we get a duplicate using some index manipulation.
Explanation
For the first image we can select any image within the set of valid indices for the array i.e. 0 to array.length - 1 which we can do using Math.floor(Math.random() * array.length). Then we store the selected index in a variable currentImageIdx.
For any following image we generate a step value which determines the amount of steps we want to move on from the current index to the next value. We need to choose this in the range of 1 to array.length - 1 which we can do by using Math.floor(1 + Math.random() * (arrayLength - 1)) as this will make sure we will never move on so far that we are back at the same index as we currently are. We then just need to add that step value to the current index and take the remainder using modulo to make sure we’re not getting out of bounds.
Examples for moving on to next index
For the following examples an array with 3 values is assumed:
We’re currently at index 0 so we should either move on 1 or 2 steps. If we were to move on 3 steps we would be at 0 again because (start index + step) % array.length would translate to (0 + 3) % 3 = 0 and we’d be at the start index again.
If we’re at index 1 we could again move on 1 or 2 steps but not 3 because (start index + step) % array.length would translate to (1 + 3) % 3 = 1 and we’d be at the start index again.
The same applies for index 2
This will work for an array of any size. Except that for the case of just having one element in the array that one element will of course be selected every time.
let currentImageIdx = undefined;
function my() {
var bt = document.getElementById("btng");
bt.textContent = "Suggest me another";
var my = new Array("https://dummyimage.com/20x20/000/fff", "https://dummyimage.com/20x20/00FF00/000", "https://dummyimage.com/20x20/FF0000/000");
let backgroundImageIdx;
// it is imporant to check for undefined here as currentImageIdx != 0 would also trigger this case if it just was if(!currentImageIdx) and may produce the same picture twice
if (currentImageIdx !== undefined) backgroundImageIdx = getNextImage(currentImageIdx, my.length)
else backgroundImageIdx = Math.floor(Math.random() * my.length);
document.getElementById("img").src = my[backgroundImageIdx];
currentImageIdx = backgroundImageIdx;
console.log(`selected image ${currentImageIdx}`);
document.getElementById(
"pic"
).style.backgroundImage = `url(${my[backgroundImageIdx]})`;
}
function getNextImage(currentIndex, arrayLength) {
// generate random step value between 1 and array.length - 1
// with three elements: either 1 and 2
var step = Math.floor(1 + Math.random() * (arrayLength - 1));
// add the step value to current index and make sure we're not out of bounds
console.log(`(${currentIndex} + ${step}) % ${arrayLength} = ${(currentIndex + step) % arrayLength}`)
return (currentIndex + step) % arrayLength;
}
function my(){
var bt=document.getElementById("btng");
bt.textContent = "Suggest me another";
var my = new Array("im/R.jpg","im/S.jpg","im/E.jpg");
var randomNum = Math.floor(Math.random() * my.length);
var backgroundImage = my[randomNum];
document.getElementById("img").src = my[randomNum];
document.getElementById("pic").style.backgroundImage=`url(${backgroundImage})`;
}
The onclick event only runs after the button is clicked twice or thrice sometimes I tried doing something to slove it but I was unsuccessful, is there any way to do it? Here is the live preview:- https://mangasuggestions.000webhostapp.com/index.html
Please click on buttons 4/5 times to get the problem I am facing.
Comments
Comment posted by Teemu
When the array is that short, it’s possible to consequentially get the same random number, and the background image isn’t changed.
Comment posted by Gaito Uchiha
Any way to prevent it? @Teemu
Comment posted by Mushroomator
It certainly gets executed. Just add a
Comment posted by Gaito Uchiha
So making a array big would slove the problem? @Mushroomator
Comment posted by Mushroomator
No making the array bigger won’t prevent it, it will just make it less likely to happen as the chance of selecting the same image twice in a row =
Comment posted by Mark Baijens
I would add a check if there are atleast 2 images in the array to prevent a never ending while loop.
Comment posted by Andrew Li
@MarkBaijens I am sorry, there are some incorrect logics. I have edited my answer and it should work now
Comment posted by m51
@AndrewLi You are not reassigning
Comment posted by Andrew Li
@m51 I am sorry, I forgot to change my code on the Main logic. The full code should be correct.
Comment posted by Gaito Uchiha
Thanks your logic worked but can you tell me the logic in more detail
