3

I am a student and very new to JS. I need to display new advert images on the page each time the users re visits the page. I was thinking of storing the img src path in an array go from there. I have got it working but the code isnt very efficient. Can anyone let me know how I could improve my code and what would be a better way of doing this? Thanks.

var content = document.getElementById("advert-content");

//storing the ads img src into an array
var adverts = ["Assets/pineapple.jpg", "Assets/conf.jpg", "Assets/balloonsad.jpg"];


function getRand() {
    return Math.floor(Math.random() * Math.floor(3));
  }

function setAd(){
    //check if the localstorage add exists and 
    if (localStorage.ad == 0){
        content.setAttribute("src", adverts[1]);
    }
    else if (localStorage.ad == 1) {
        content.setAttribute("src", adverts[2]);
    }
    else if (localStorage.ad == 2) {
        content.setAttribute("src", adverts[0]);
    }
}

function update(){
      // if nothing is stored in the local storage display the first advert image
    if (!localStorage.ad){
        content.setAttribute("src", adverts[0]);
    }
    else {
        setAd();
        let randnum = getRand();
        //get a different number than the one stored inside the local storage

        while (randnum == localStorage.ad) {
            randnum = getRand();
        }
        // update the image with a different number
        localStorage.ad = adverts.indexOf(adverts[randnum]);
    }
}

//call function
update();
 <section id = "adverts">
    <img id="advert-content"/>
 </section>
cssyphus
  • 37,875
  • 18
  • 96
  • 111
  • store the image path to where? the client's machine? server-side? If you are going to store it for something with an absolute url (`http://example.com`).. why? It's already on the internet. If you're going to store it on the client side... why? Especially if what it appears to be doing (showing ads) users can just wipe their local storage – soulshined Feb 22 '19 at 20:42
  • Its part of my school assignment. I wouldnt store different ads on local storage but i am is required to in this case. – Mantas Stanionis Feb 22 '19 at 22:12

1 Answers1

0

Here are a few improvements to your code:

var storage = 'localStorage'; // we can change between storages if required
var adLabel = 'ad'; // another string we might need to update someday

var content = document.getElementById("advert-content");

//storing the ads img src into an array
var adverts = ["https://placeholdit.imgix.net/~text?txtsize=63&bg=F64&txtclr=fff&txt=Image-2", "https://placeholdit.imgix.net/~text?txtsize=63&bg=F64&txtclr=fff&txt=Image-3", "https://placeholdit.imgix.net/~text?txtsize=63&bg=F64&txtclr=fff&txt=Image-4"];


function getRand(a) {
    // let's use the array length to avoid updating this function
    return Math.floor(Math.random() * Math.floor(a.length));
}

function setAd(adNum){
    // just set the src and storage value
    content.setAttribute("src", adverts[adNum]);
    // save only the index instead of the whole url string
    window[storage].setItem(adLabel, adNum);
}

function update(){
    let randNum;
    //get a different number than the one stored inside the local storage
    let localStorageAd = window[storage].getItem(adLabel);
    do {
        randNum = getRand(adverts);
    } while (randNum == localStorageAd)
    // update the image with a different number
    setAd(randNum);
}

//call function
update();
<section id = "adverts">
    <img id="advert-content" src="https://placeholdit.imgix.net/~text?txtsize=63&bg=F64&txtclr=fff&txt=Image-1"/>
 </section>
rafaelcastrocouto
  • 11,781
  • 3
  • 38
  • 63