Javascript execution order when using if statements

Question

I am a novice here, I apologize for any ignorance. I am trying to create a family feud game for personal use using node.js and express as the server and socket.io for communication between clients. I ran into an issue with this particular function where if there are less than 5 players it will include all but the last one in the socket message. For example if there are 3 players it seem to be running the player4 else statement (for when blank) before completing the player3 variable assignment. I have tried researching it and found promises and async/await but I don't understand it enough to make it work for my situation or if it would even apply. This is a lot of nested if statements. I feel like a loop would be better but not sure how to do it properly. Any help and suggestions are much appreciated.

if (data.teamNum == 'team1') {
    team1.teamNum = 'team1';
    fs.access('./public/images/players/' + data.player1, function(error) {
      if (error) {
        console.log(data.player1 + " icons do not exist.")
        team1.player1pic = "default"
      } else {
        console.log(data.player1 + " icons exist.")
        fs.readdir('./public/images/players/' + data.player1, (error, files) => { 
          team1.player1pic = files; // return the number of files
          console.log(data.player1 + " has " + team1.player1pic + " pics.");
        });
      }
      if (data.player2 != 'blank') {
        fs.access("./public/images/players/" + data.player2, function(error) {
          if (error) {
            console.log(data.player2 + " icons do not exist.")
            team1.player2pic = "default"
          } else {
            console.log(data.player2 + " icons exist.")
            fs.readdir('./public/images/players/' + data.player2, (error, files) => { 
              team1.player2pic = files; // return the number of files
              console.log(data.player2 + " has " + team1.player2pic + " pics.");
            });
          }
          if (data.player3 != 'blank') {
            fs.access("./public/images/players/" + data.player3, function(error) {
              if (error) {
                console.log(data.player3 + " icons do not exist.")
                team1.player3pic = "default"
              } else {
                console.log(data.player3 + " icons exist.")
                fs.readdir('./public/images/players/' + data.player3, (error, files) => { 
                  team1.player3pic = files; // return the number of files
                  console.log(data.player3 + " has " + team1.player3pic + " pics.");
                });
              }
              if (data.player4 != 'blank') {
                fs.access("./public/images/players/" + data.player4, function(error) {
                  if (error) {
                    console.log(data.player4 + " icons do not exist.")
                    team1.player4pic = "default"
                  } else {
                    console.log(data.player4 + " icons exist.")
                    fs.readdir('./public/images/players/' + data.player4, (error, files) => { 
                      team1.player4pic = files; // return the number of files
                      console.log(data.player4 + " has " + team1.player4pic + " pics.");
                    });
                  }
                  if (data.player5 != 'blank') {
                    fs.access("./public/images/players/" + data.player5, function(error) {
                      if (error) {
                        console.log(data.player5 + " icons do not exist.")
                        team1.player5pic = "default"
                      } else {
                        console.log(data.player5 + " icons exist.")
                        fs.readdir('./public/images/players/' + data.player5, (error, files) => { 
                          team1.player5pic = files; // return the number of files
                          console.log(data.player5 + " has " + team1.player5pic + " pics.");
                          console.log('sending pics');
                          feud.in(data.room).emit('teampics', team1);
                      });
                      }
                    });
                  } else {
                    console.log('sending pics');
                    feud.in(data.room).emit('teampics', team1);
                  }
                });
              } else {
                console.log('sending pics');
                feud.in(data.room).emit('teampics', team1);
              }
            });
          } else {
            console.log('sending pics');
            feud.in(data.room).emit('teampics', team1);
          }
        });
      } else {
        console.log('sending pics');
        feud.in(data.room).emit('teampics', team1);
      }
    });
  }

Yes, try using promises and async/await, but on your way to getting there you should rewrite this by a) using a helper function to remove the duplication b) using a recursive approach to do it for 5 players — Bergi
– Bergi, Commented Jul 15, 2020 at 17:18
What you probably want is already created in this FWK colyseus.io . While a noble endeavour, you should always avoid re-inventing the wheel. — Ivo Monti
– Ivo Monti, Commented Jul 15, 2020 at 17:31
@IvoMonti thank you. I will definately check it out. I wish I found that a year ago when I started this thing. :( — andrewjae04
– andrewjae04, Commented Jul 15, 2020 at 20:24

Molda · Accepted Answer · 2020-07-15 19:53:41Z

2

I hope the code is self explanatory. I haven't tested the getPlayerPic function. I used callback('default') for the test.

If you have to write same code more then twice always make it a function which you run multiple times. The code will be much easier to read as well as easier to find the problem.

// function to get the player picture
function getPlayerPic(player, callback) {
    //return callback('default' + player);
    fs.access('./public/images/players/' + player, function(error) {
        if (error) return callback('default');
        fs.readdir('./public/images/players/' + data.player1, (error, files) => {
            callback(files);
        });
    });
};

// test data
var data = {
    teamNum: 'team1',
    player1: 'p1',
    player2: 'p2',
    player3: 'p3',
    player4: 'blank',
    player5: 'blank',
}

var team1 = {};

if (data.teamNum == 'team1') {
    team1.teamNum = 'team1';

    var players = Object.keys(data).filter(p => p.startsWith('player')).reverse();
    // players = [ 'player1', 'player2', 'player3', 'player4', 'player5' ];


    function next(callback) {
        var p = players.pop(); // pop an item from players
      console.log('p', p);
        if (p && data[p] && data[p] !== 'blank') // if it exists and is not blank
            getPlayerPic(data[p], function(pic){
                team1[p + 'pic'] = pic;
                next(callback);
            });
        else // we are done here
            callback();
    };

    next(function(){
        console.log('sending pics', team1);
        /*
         sending pics { teamNum: 'team1',
           player1pic: 'defaultp1',
           player2pic: 'defaultp2',
           player3pic: 'defaultp3' }
        */
    });
}

edited Jul 15, 2020 at 19:53

answered Jul 15, 2020 at 19:22

Molda

5,7142 gold badges26 silver badges42 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

andrewjae04 Over a year ago

Thank you for the assistance. I am still learning and trying to work through the code, some of this is over my head. To further make use of the answer you provided, data.teamNum can also have a 'team2' option. to keep from creating an else and repeating the code I can set the team1.teamNum variable outside of the function but how can I pass in the teamNum as a parameter of the function so that it sets the pic's to the proper object (team1 or team2)?

Molda Over a year ago

Just pass it to next function: function next(team, callback) and call it inside next function like this next(team, callback); and in the first call use next(teamX, function(){ And instead of team1[p + 'pic'] = pic; use team[p + 'pic'] = pic;

Cat · Accepted Answer · 2020-07-15 20:58:17Z

Here's a more functional approach to the same problem.
(If you become enamored of functional programming, check out JS Allongé.)

Caveat rogantis: This code is completely untested, and I'd bet money it will require debugging. I didn't look into how fs.access and fs.addredir work so it may also need to be refactored to make the callbacks legal.

const { teamNum, room, player1, player2, player3, player4, player5 } = data;
const playersList = [player1, player2, player3, player4, player5];
let done;

// We're only dealing with team1 for now  
const team1 = {};
let team = team1;
team.teamNum = teamNum;
 
// Assigns icons for each player (stops if a blank player is found)
for(let player of playersList){
  assignIcons(player);
  if(isLastPlayer(player)){
    break;
  }
}

// Primary functions
function assignIcons(player){
  fs.access(getPath(player), accessCallback(error, player));
}

function accessCallback(error, player){
  if (error) {
    // Logs "player has no icons" and assigns default
    logPlayerHasIcons(player, false);
    assignIconCount(player, "default");
  }
  else {
    // Logs "player has icons" and gets the count
    logPlayerHasIcons(player, true);
    fs.readdir(getPath(player), readdirCallback(error, fileCount, player);
  }
}

function readdirCallback(error, fileCount, player){

  // Assigns the icon count for this player
  if(error){ console.log(error); }
  assignIconCount(player, fileCount);
  logIconCount(getPlayerNum(player), fileCount);

  // Emits pics if done
  if(isLastPlayer(player)){
    emitPics();
  }
}

function emitPics(){
  console.log('sending pics');
  feud.in(room).emit('teampics', team);
}

function assignIconCount(player, val){
  const
    playerNum = getPlayerNum(player),
    playerPicPropName = `player${playerNum}pic`;
  team[playerPicPropName] = val;
}

// Supporting functions
function isLastPlayer(player){
  const playerNum = getPlayerNum();
  return (
    (playerNum == 5) ||
    (playersList[playerNum] == "blank") // playerNum = index of next player
  );
}

function getPlayerNum(player){
  return playersList.indexOf(player) + 1;
}

function getPath(player){
  return "./public/images/players/" + player;
}

function logPlayerHasIcons(player, doesHaveIcons){
  console.log(`${player} ${(doesHaveIcons ? "has" : "has no")} icons`);
}

function logIconCount(playerNum, count){
  console.log(`player${playerNum} has ${count} pics`);
}

Collectives™ on Stack Overflow

Javascript execution order when using if statements

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related