JavaScript change image every three seconds I have a function that changes the image on a webpage every three seconds. I would like feedback on efficiency, good code practices, and anything else that can improve the quality of this code. It works, I just want any insight if it can work any better. Any and all feedback is welcomed and considered.

Slideshow Script

&lt;!-- Script for Slide Show: Change picture every three seconds --&gt;
&lt;script type="text/javascript"&gt;
    var index = 0;
    change();

    function change() {

        //Collect all images with class 'slides'
        var x = document.getElementsByClassName('slides');

        //Set all the images display to 'none' (invisible)
        for(var i = 0; i &lt; x.length; i++) { 
            x[i].style.display = "none"; 
        }

        //Increment index variable
        index++;

        //Set index to 1 if it's greater than the amount of images
        if(index &gt; x.length) { 
            index = 1; 
        }

        //set image display to 'block' (visible)
        x[index - 1].style.display = "block";

        //set loop to change image every 3000 milliseconds (3 seconds)
        setTimeout(change, 3000);
    }
&lt;/script&gt;


HTML Images accessed

&lt;!-- Slide Show --&gt;
&lt;section&gt;
    &lt;img class="slides" src="external/ph1.jpg" style="width:100%"&gt;
    &lt;img class="slides" src="external/ph2.jpg" style="width:100%"&gt;
    &lt;img class="slides" src="external/ph3.jpg" style="width:100%"&gt;
&lt;/section&gt;
The script tag default type is text/javascript and thus is not needed.
Keep your script out of the global scope. This can become a problem when more JS is added to the page and you get name conflicts. The easiest way to encapsulate your code is to wrap it in a function. In this case it would be best to wait for the page to load before starting image cycle. You can use either DOMContentLoaded or load events but if you use the first be sure that the CSS has loaded.
Drop the comments they add nothing that is not self evident in the code itself, and are but noise. If you find that the code does not contain the information needed to understand it, first can you rename to make it clear, if not then comments are the last resort.
Define a CSS class for hidden images rather than setting the style properties directly. (see example)
Use the CSS class to define style properties. Do not set style properties in the HTML document. Its just easier to manage.
Use querySelectorAll it is more flexible in the long run.
Use for of loop rather than for(;;) loops as it cleaner and requires less code.
You don't need to query for the images every 3 seconds. Do it once at the start.
Use const for variables that do not change.
The variable name x is a bad name. slideImages of slides would be more fitting. Note that I use the plural signifying that it is array, or array like.
Start with the images hidden, then ever 3 seconds all you need to do is hide one and show one.
Use the remainder operator % to cycle a value (see example)


Example

&lt;style&gt;
    .slides { width: 100%; }
    .slides-hidden { display : none; }
&lt;/style&gt;

&lt;script&gt;
    addEventListener("load",() =&gt; { // "load" is safe but "DOMContentLoaded" starts earlier
        var index = 0;
        const slides = document.querySelectorAll(".slides");
        const classHide = "slides-hidden", count = slides.length;
        nextSlide();
        function nextSlide() {
            slides[(index ++) % count].classList.add(classHide);
            slides[index % count].classList.remove(classHide);
            setTimeout(nextSlide, 3000);
        }
    });
&lt;/script&gt;

&lt;section&gt;
    &lt;img class="slides slides-hidden" src="external/ph1.jpg"&gt;
    &lt;img class="slides slides-hidden" src="external/ph2.jpg"&gt;
    &lt;img class="slides slides-hidden" src="external/ph3.jpg"&gt;
&lt;/section&gt;