How can i avoid the don't repeat your self rule with this function

49 views Asked by At

He Developers,

I want to know your opinion about how to wright this code better.

If you click on button star3 it will turend as a yellow star stars underneath are also yellow. The star above star3 is just grey.

Thats what i got and what i need but i want to know how i can be more efficient in my code..

var star = document.getElementById("star-0");
 var star1 = document.getElementById("star-1");
 var star2 = document.getElementById("star-2");
 var star3 = document.getElementById("star-3");
 var star4 = document.getElementById("star-4");
 var img = 'url(img/star.png)';
 var img2 = 'url(img/star2.png)';


star4.addEventListener('click', function(){
   star.style.backgroundImage = img2;
   star1.style.backgroundImage = img2;
   star2.style.backgroundImage = img2;
   star3.style.backgroundImage = img2;
   star4.style.backgroundImage = img2;
});

star3.addEventListener('click', function(){
   star.style.backgroundImage = img2;
   star1.style.backgroundImage = img2;
   star2.style.backgroundImage = img2;
   star3.style.backgroundImage = img2;
   star4.style.backgroundImage = img;
});

star2.addEventListener('click', function(){
   star.style.backgroundImage = img2;
   star1.style.backgroundImage = img2;
   star2.style.backgroundImage = img2;
   star3.style.backgroundImage = img;
   star4.style.backgroundImage = img;
});

star1.addEventListener('click', function(){
   star.style.backgroundImage = img2;
   star1.style.backgroundImage = img2;
   star2.style.backgroundImage = img;
   star3.style.backgroundImage = img;
   star4.style.backgroundImage = img;
});

star.addEventListener('click', function(){
   star.style.backgroundImage = img2;
   star1.style.backgroundImage = img;
   star2.style.backgroundImage = img;
   star3.style.backgroundImage = img;
   star4.style.backgroundImage = img;
});

3

There are 3 answers

1
MaX On BEST ANSWER

Seems like you are trying to implement rating stars. If you insist on only using JS not using any smarts or meta-data in HTML. Here is the function I would simplify it to:

var star = document.getElementById("star-0");
 var star1 = document.getElementById("star-1");
 var star2 = document.getElementById("star-2");
 var star3 = document.getElementById("star-3");
 var star4 = document.getElementById("star-4");
 var img = 'url(img/star.png)';
 var img2 = 'url(img/star2.png)';

function updateRating(rating) {
  var stars = [star, star1, star2, star3, star4];
  for(var i = 0; i < stars.length; i++) {
    var appliedImage = img;
    if ((i + 1) < rating) {
      appliedImage = img2;
    }
    
    stars[i].style.backgroundImage = appliedImage;
  }
}


star4.addEventListener('click', function(){
   updateRating(5);
});

star3.addEventListener('click', function(){
   updateRating(4);
});

star2.addEventListener('click', function(){
   updateRating(3);
});

star1.addEventListener('click', function(){
   updateRating(2);
});

star.addEventListener('click', function(){
   updateRating(1);
});

0
Pabs123 On

Give all of the stars a common class like class="stars". Then do:

var star1 = document.querySelector('.stars');

From there you can attach an event listener to them instead of writing each one out individually.

Inside of the click listener, make the background of the current event target's and all the ones with an id lower than itself be img2.

0
Lockless On

Use one function to add all the stars and call it with the click event. Pass in the id of the star. Check with if statements and stop when it matches.