CODEWITHSUNDEEP
javascriptrefactoring
Simonsoft177
Simonsoft177

Reputation: 175

Javascript - How to refactor a code to look cleaner

I have a function that looks kind of primitive, I am wondering whether anyone has any solution on how to improve the looks of this function. Can I change this primitive loop if()... if()... to something that looks cleaner and nicer?

function drawPlayers () {
    if (players[0].nick != null) {
        let player1Img = new Image(SQUARE, SQUARE)
        player1Img.onload = function() {
            ctx.drawImage(player1Img, LEFT_LINE + players[0].x * SQUARE, UPPER_LINE + players[0].y * SQUARE, this.width, this.height)
        }
        player1Img.src = "sprites/player1.png"
    }
  
    if (players[1].nick != null) {
        let player2Img = new Image(SQUARE, SQUARE)
        player2Img.onload = function() {
            ctx.drawImage(player2Img, LEFT_LINE + players[1].x * SQUARE, UPPER_LINE + players[1].y * SQUARE, this.width, this.height)
        }
        player2Img.src = "sprites/player1.png"
    }
  
    if (players[2].nick != null) {
        let player3Img = new Image(SQUARE, SQUARE)
        player3Img.onload = function() {
            ctx.drawImage(player3Img, LEFT_LINE + players[2].x * SQUARE, UPPER_LINE + players[2].y * SQUARE, this.width, this.height)
        }
        player3Img.src = "sprites/player1.png"
    }
  
    if (players[3].nick != null) {
        let player4Img = new Image(SQUARE, SQUARE)
        player4Img.onload = function() {
            ctx.drawImage(player4Img, LEFT_LINE + players[3].x * SQUARE, UPPER_LINE + players[3].y * SQUARE, this.width, this.height)
        }
        player4Img.src = "sprites/player1.png"
    }
}

Upvotes: 0

Views: 90

Answers (4)

Yevhenii Shlapak
Yevhenii Shlapak

Reputation: 786

Another variant:

for(let p of players){
  if(p.nick){
    let playerImg = new Image(SQUARE,SQUARE);
    playerImg.onload = function() {
        ctx.drawImage(player1Img, LEFT_LINE + p.x*SQUARE, UPPER_LINE + p.y*SQUARE, this.width, this.height)
    }
    playerImg.src = "sprites/player1.png"
  }
}

Another feature, I learned recently:

for(let p of players){
  if(!p.nick) continue;
  let playerImg = new Image(SQUARE,SQUARE);
  playerImg.onload = function() {
     ctx.drawImage(player1Img, LEFT_LINE + p.x*SQUARE, UPPER_LINE + p.y*SQUARE, this.width, this.height)
  }
  playerImg.src = "sprites/player1.png"
}

Upvotes: 2

mplungjan
mplungjan

Reputation: 177786

Like this

players.forEach(player => {
  if (player.nick != null) {
    let img = new Image(SQUARE, SQUARE)
    img.onload = function() {
      ctx.drawImage(img, LEFT_LINE + player.x * SQUARE, UPPER_LINE + player.y * SQUARE, this.width, this.height)
    }
    img.src = "sprites/player1.png"; // or `sprites/${player.image}`;
  }
});

If you have another array for image names you can add an index to the forEach :

players.forEach((player,i) => {
  if (player.nick != null) {
    let img = new Image(SQUARE, SQUARE)
    img.onload = function() {
      ctx.drawImage(img, LEFT_LINE + player.x * SQUARE, UPPER_LINE + player.y * SQUARE, this.width, this.height)
    }
    img.src = `sprites/${images[i]}`;
  }
});


const SQUARE = 100;
const images = [
  "https://via.placeholder.com/100x100?text=Image1",
  "https://via.placeholder.com/100x100?text=Image2",
  "https://via.placeholder.com/100x100?text=Image3"
];
const players = [
{ nick: "Fred", x: 10, y: 20 },
{ nick: "Joe", x: 20, y: 40 },
{ nick: "Sam", x: 30, y: 50 }
];



players.forEach((player, i) => {
  if (player.nick != null) {
    let img = new Image(SQUARE, SQUARE)
    img.onload = function() {
      console.log(i,player.nick,`ctx.drawImage(img, LEFT_LINE ${player.x} * SQUARE, UPPER_LINE + ${player.y} * SQUARE, ${this.width}, ${this.height})`)
    }
    img.src = `${images[i]}`;
  }
});

Upvotes: 4

iAmOren
iAmOren

Reputation: 2804

function drawPlayers() {
  players.forEach((player, idx) => {
    if (player.nick != null) {
      // uncomment following comment block and delete this comment
      /*
      var img = new Image(SQUARE, SQUARE)
      img.onload = () => {
        ctx.drawImage(img, LEFT_LINE + player.x * SQUARE, UPPER_LINE + player.y * SQUARE, this.width, this.height)
      };
      img.src = "sprites/player"+(idx+1)+".png";
      */
      console.log(idx, player.nick, "sprites/player"+(idx+1)+".png");
    }
  });
}

var players=[{nick:"abe"},{},{nick:"chuck"},{nick:"dick"}];
drawPlayers();

Upvotes: 2

hgb123
hgb123

Reputation: 14891

You could do a for loop

function drawPlayers() {
  for (let i = 0; i < players.length; i++) {
    if (players[i].nick != null) {
      let playerImg = new Image(SQUARE, SQUARE)
      playerImg.onload = function() {
        ctx.drawImage(
          playerImg,
          LEFT_LINE + players[i].x * SQUARE,
          UPPER_LINE + players[i].y * SQUARE,
          this.width,
          this.height
        )
      }
      // if the image is fixed
      playerImg.src = 'sprites/player1.png'
      // else
      // playerImg.src = `sprites/player${i + 1}.png`
    }
  }
}

Upvotes: 3

Related Questions