Cleaning up the Arrays
I second JamesBodgan's suggestion to store only the image src
attributes and link href
attributes in your arrays instead of storing string representations of the HTML.
This will change the way you consume the values from the arrays. You might want to create a couple functions that create the actual HTML elements from the given attributes for you.
function createLink(url,innerElement){
let a = document.createElement("a");
a.setAttribute("href",url);
a.appendChild(innerElement);
return a;
}
function createImage(src){
let img = document.createElement("img");
img.setAttribute("src",src);
return img;
}
I'll also second James's suggestion to rename those arrays to images
and links
to reflect their plurality.
Capture Element IDs and Min/Max Values in a Data Structure
Currently, you have the min/max values and element IDs directly in the code wherever they're needed. You can make the code easier to maintain by moving those values to a separate, dedicated data structure.
I recommend using an array of objects. Then you can loop through the array to perform the necessary operations, which makes adding more elements/images/links almost trivial.
let elements = [
{id:'#pos-1',min:0,max:1},
{id:'#pos-2',min:1,max:3},
{id:'#pos-3',min:3,max:5},
];
for(let i = 0, len = elements.length; i < len; i++){
let currElement = elements[i];
let x = Math.floor(Math.random()*(currElement.max - currElement.min) + currElement.min);
let link = createLink(links[x],createImage(images[x]));
$(currElement.id).append(link);
}
This also eliminates some repetition from your original code, which is always a good sign.
Note that for improved DOM-query efficiency, you could use the native JavaScript document.getElementById()
instead of the generic $()
jQuery selector function.
document.getElementById(currElement.id).appendChild(link);
You'd just need to be sure you remove the #
prefix from the element IDs.
Do you need jQuery?
At this point, you might want to consider whether you need jQuery at all.
Odds are good that there are other things on the page that are using jQuery, so eliminating it entirely may be unfeasible. But there can be a benefit to minimizing your use of jQuery even on pages where it's being loaded regardless.
The native, underlying JavaScript methods that are invoked by jQuery wrapper functions will always be more efficient (in terms of how quickly the browser executes them), at the expense of being slightly more verbose.
var images = ['https://placehold.it/200/09f/fff.png?text=image1',
'https://placehold.it/200/000/fff.png?text=image2',
'https://placehold.it/200/22f/000.png?text=image3',
'https://placehold.it/200/32f/fff.png?text=image4',
'https://placehold.it/200/42f/fff.png?text=image5'
];
var links = [
"http://www.jquery.com",
"http://www.microsoft.com",
"http://www.yahoo.com",
"http://www.msn.com",
"http://www.stackoverflow.com"
];
let elements = [{
id: 'pos-1',
min: 0,
max: 1
}, {
id: 'pos-2',
min: 1,
max: 3
}, {
id: 'pos-3',
min: 3,
max: 5
}, ];
for (let i = 0, len = elements.length; i < len; i++) {
let currElement = elements[i];
let x = Math.floor(Math.random() * (currElement.max - currElement.min) + currElement.min);
let link = createLink(links[x], createImage(images[x]));
document.getElementById(currElement.id).appendChild(link);
}
function createLink(url, innerElement) {
let a = document.createElement("a");
a.setAttribute("href", url);
if (!innerElement) {
innerElement = document.createTextNode(url);
}
a.appendChild(innerElement);
return a;
}
function createImage(src) {
let img = document.createElement("img");
img.setAttribute("src", src);
return img;
}
<div id='pos-1' style='margin-top: 1em;'>
</div>
<div id='pos-2' style='margin-top: 1em;'>
</div>
<div id='pos-3' style='margin-top: 1em;'>
</div>
I find that the native equivalents to jQuery wrappers tend to be more explicitly and precisely named, making it easier for me to recognize mistakes and intentions and making my code more maintainable in the long run.