patternjavascriptMinor
Constructing a JavaScript slide
Viewed 0 times
javascriptconstructingslide
Problem
I am a newbie to native JavaScript and am pretty much coming to grips with the operators, etc. At the moment I am currently constructing a slide.
I am not a fan of the
"use strict";
var intSeconds = 3;
function loadSlider(){
flickrImgsShown = 0;
sliderImages = new Array(slides + 10);
sliderImages[0] = document.querySelector(".img");
for (var i = 1; 1 < sliderImg; i++ ) {
sliderImages[i] = document.querySelector(".img" + (i + 1));
document.querySelector(".identifer" + i);
}
function next() {
flickrImgsShown++;
if(flickrImgsShown == slides)
flickrImgsShown = 0;
transition();
clearInterval(slideTimer);
slideTimer = setInterval(timer, interSeconds * 1000);
}
function prev () {
flickrImgsShown--;
if(flickrImgsShown == -1)
flickrImgsShown = slides -1;
transition();
clearInterval(slideTimer);
slideTimer = setInterval(timer, interSeconds * 1000);
}
slideTimer = setInterval(timer, intSeconds * 1000);
function timer() {
flickrImgsShown--;
if(flickrImgsShown == -1)
flickrImgsShown = slides -1;
transition();
}
}I am not a fan of the
getelementbyidand style.visibility in my code. Is there a better method I can apply to call out an ID or change a style in the DOM?Solution
Your questions seem to have been answered (use jQuery), so here are a couple of small points regarding readability:
- be consistent with your variable names (eg
sliderImgvsflickrImgs: one is plural, one singular, and in the rest of the code, you mainly useflickrX, except for thesliderImgone; I would prefersliderXeverywhere, because it doesn't seem to matter where the images come from).
- some variable names could be a lot better. Eg when reading
flickrImgs/sliderImgI'm thinking it's an array of images (or image paths/names). But they are not, they are integers, so I would preferimageCount/imageSize, andcurrentSliderPosition.flickrDisplayon the other hand seems to actually be a collection of images, sosliderImageswould probably be a good name for it.
- your indentation is off, which decreases the readability of your code.
- your comments aren't all that helpful.
NOW AND NEXT FUNCTION/SLIDE TRANSITIONaren't really necessary, andFor each loop incrementing each slide by onedoesn't really help either (because that's exactly what the code already told me; If you do want to add a comment, it would be more helpful to explain what it means when a slide is increased.
- use curly brackets even for one line statements to increase readability and to possibly avoid bugs.
visiableisn't a valid value forvisibility, you probably meantvisible.
Context
StackExchange Code Review Q#83640, answer score: 3
Revisions (0)
No revisions yet.