Can this be improved, or is it good?
How it works is, you click on the blue play image and a random video starts playing.
Is there anything you would change in how the code is written?
https://jsfiddle.net/9amy345p/
https://jsitor.com/N3p4XhfLY1
const videos = thewrap.querySelectorAll("div.jacket");
const random = Math.floor(Math.random() * videos.length);
videos[random].click();
javascript:
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
const videos = thewrap.querySelectorAll("div.jacket");
const random = Math.floor(Math.random() * videos.length);
videos[random].click();
}
const cover = document.querySelector(".play");
cover.addEventListener("click", coverClickHandler);
}());
This is the code that is used to have a random video play.
Is that good?
const videos = thewrap.querySelectorAll("div.jacket");
const random = Math.floor(Math.random() * videos.length);
videos[random].click();
Does anyone on here think the code should be written a different way?
I was told to do this, but I don’t know how it would be written into my code.
Extract the random number generation into a separate function like rand = (max, min = 0) => Math.floor(Math.random() * (max - min)) + min;
so it’s earsier to re-use videos[rand(videos.length)]
.
I’m having trouble figuring out how to write it into the code.
I tried this:
function getRandom(max, min = 0) {
const videos = thewrap.querySelectorAll("div.jacket");
const random = Math.floor(Math.random) * (max - min)) + min.click();
}
the .click() method is only applied to the videos[random].click(), so do not apply it. What I understand is your max must be as such: max = videos.length, while min = 0.
How do you not apply it?
How would it be written without .click()
?
What do you mean when you say so do not apply it?
Something like this?
function newRandomNumber(min, max) {
return Math.floor(Math.random() * max) + min;
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
newRandomNumber(0, 3).click();
}
or maybe like this?
function newRandomNumber(min, max) {
return Math.floor(Math.random() * (max - min + 0 ))) + min;
}
like this?
function newRandomNumber(min, max) {
const videos = thewrap.querySelectorAll("div.jacket");
return Math.floor(Math.random() * videos.length);
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
videos[random].click();
}
I don’t understand how it would be written.
I only know of one way of writing the code, I was given 2 suggestions of other ways it could be written instead.
One way is putting the random code into it’s own function, and another is
using an array which would look something like this I think.
const videos = ['.jacket-left', '.jacket-middle', 'jacket-right'];
I would like to know and be able to figure these ways out on how to write those.
You’re overthinking this. The advice given to you was to replace this line
const random = Math.floor(Math.random() * videos.length);
with this
const random = rand(videos.length);
And then add the rand
function definition somewhere else in the file or project.
That’s all.
``rand function definition
How is that written?
function rand(min, max) {
const videos = thewrap.querySelectorAll("div.jacket");
return Math.floor(Math.random() * videos.length);
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
const random = rand(videos.length);
videos[random].click();
}
The person giving you advice showed you already, but here’s another example.
The Math.random() static method returns a floating-point, pseudo-random number that's greater than or equal to 0 and less than 1, with approximately uniform distribution over that range — which you can then scale to your desired range. The...
No, this is not generic. The point is to make a generic function that does nothing but return a random integer between two values. It should not know about videos or divs or anything outside the scope of returning a random integer. That way, it can be reused easily in other parts of the code where you find yourself needing a random integer.
Like this then?
https://jsfiddle.net/npt65fjv/
function rand(max) {
return Math.floor(Math.random() * max);
}
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
const videos = thewrap.querySelectorAll("div.jacket");
const random = rand(videos.length);
videos[random].click();
}
1 Like
Yes, like that. Good job.
1 Like
When someone suggested this, what did they mean?
I’d make it accept an array, and return a random item from it.
One of these would be used, and then what?
const videos = ['.jacket-left', '.jacket-middle', 'jacket-right'];
const videos = ['9phZWySNsXY ', 'qe5WF4qCSkQ', 'SiGnAi845o'];
function randomInt(max) {
return Math.floor(Math.random() * max);
}
function randomItemFrom(array) {
const randomIndex = randomInt(array.length);
return array[randomIndex];
}
const someArray = [1, 2, 3, 4, 5];
const randomItemFromArray = randomItemFrom(someArray);
What would be removed and replaced in here?
https://jsfiddle.net/5cf8vsr9/1/
function randomInt(max) {
return Math.floor(Math.random() * max);
}
function randomItemFrom(array) {
const randomIndex = randomInt(array.length);
return array[randomIndex];
}
const someArray = ['.jacket-left', '.jacket-middle', 'jacket-right'];
const randomItemFromArray = randomItemFrom(someArray);
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
const videos = thewrap.querySelectorAll("div.jacket");
const random = randomInt(videos.length);
videos[random].click();
}
javascriptcoding5678:
videos[random]
This evaluates to a random item from the videos
array. So replace this with a function call to randomItemFrom()
, passing the videos
array as the first argument.
1 Like
First thing I do is this:
const videos = thewrap.querySelectorAll("div.jacket");
const random = randomInt(videos.length);
randomItemFrom().click();
You don’t need to generate a random number anymore. You want a random item from the videos array, right?
If that’s how it works using an array, then yes, I want a random item.
function randomItemFrom(array) {
const randomIndex = randomInt(array.length);
return array[randomIndex];
}
This function return a random item from the array you pass to it. So randomItemFrom(videos)
would return a random item from videos
.
1 Like
This line would get deleted:
/* const random = randomInt(videos.length); */
Like this?
or does that line stay?
or was I supposed to do something else?
Did I do something wrong?
https://jsfiddle.net/j7h9u6bp/1/
function randomInt(max) {
return Math.floor(Math.random() * max);
}
function randomItemFrom(videos) {
const randomIndex = randomInt(videos.length);
return videos[randomIndex];
}
const someArray = ['.jacket-left', '.jacket-middle', 'jacket-right'];
const randomItemFromArray = randomItemFrom(someArray);
function coverClickHandler(evt) {
const cover = evt.currentTarget;
hide(cover);
const thewrap = cover.parentElement.querySelector(".container");
show(thewrap);
const videos = thewrap.querySelectorAll("div.jacket");
/*const random = randomInt(videos.length);*/
randomItemFrom(videos).click();
}