I have written the following function to empty and then populate multiple elements using jQuery:
function drawHumanCard()
{
var random = generateRandom();
var humanEl = $('#human');
humanEl.find('#human-name').empty().text(cards[random]['name']);
humanEl.find('#human-job').empty().text(cards[random]['job']);
humanEl.find('#human-image').attr('src', 'img\/trumps\/' + cards[random]['image']);
}
var cards = [
new TopTrumpCard('Name 1', 'designer', 'image1.jpg'),
new TopTrumpCard('Name 2', 'designer', 'image2.jpg'),
new TopTrumpCard('Name 3', 'designer', 'image3.jpg')
];
function TopTrumpCard(name, job, image)
{
this.name = name;
this.job = job;
this.image = image;
}
function generateRandom()
{
var objCount = Object.keys(cards).length - 1;
var number = Math.floor(Math.random() * (objCount - 0 + 1)) + 0;
return number;
}
What the drawHumanCard
function does:
var random
is a randomly generated number between a specific range. The range is 0
and number of cards
array keys.
cards
is a variable that contains an array of data. The random
variable is used to target a specific key in the cards
array.
From then on, I've defined 3 elements. 2 of which require the text to be changed and 1 that requires the image src
attribute value to be changed.
What would be the best way to write this in a cleaner way?
I was thinking about defining the jQuery element objects in an array and then running an each
loop.
As you can see, I am using jQuery for this, but am happy to go with vanilla JS too.
generateRandom
anywhere and your variablecards
isn't defined either. Also, you don't show how you will call the functiondrawHumanCard()
. Those are essential things for a complete review. Also, your new title isn't that explicit, since it doesn't describe the purpose of the code. \$\endgroup\$generateRandom(0)
, that's exactly what it is doing. Right? And what data exactly is in thecards
array? You don't need to show the whole array. Just 3-4 elements is enough. Also// ... and so on
tells me that there's still missing code there. If you need to ask how you can rewrite this, without bloating the comment section, feel free to go on The 2nd Monitor chatroom \$\endgroup\$generateRandom()
. also, you should have your whole code in a single block. \$\endgroup\$