Array of hosted games

Question

I have an array which handles all the games being hosting on the server. Users can host games and cancel the host as they please. What I have below seems to work, but I'm not sure if it adds the object to the lowest free index, but it adds them and removes just fine. The code is a bit of a mess so I've commented a lot to help you understand.

var hostingGames = [];


server.em.addListener('hostGame', function(settings)
{   
    if (hostingGames.length != 0) //If the array that contains all the games being hosted, contains at least 1 game
        {
            for (i in hostingGames)//Scroll through every game being hosted
            {
                if(hostingGames[i] != null)//If the current index in the array does contain a game being hosted
                {
                    if(hostingGames[i].userId === settings.userId)//If the user thats trying to host a game, is already hosting a game
                    {
                        break; //Then break
                    }
                    else if (i == (hostingGames.length - 1))//If hes not hosting a game and all the games have finished being scrolled through
                    {
                        hostGame(settings, i + 1); //Then host one in the next avaliable space
                    }
                }
                else //If the game is null
                {
                    for(var i = 0; i < hostingGames.length; i++) //Scroll through the list of the games being hosted again
                    {
                        if(hostingGames[i] === null) //If theres a free slot
                        {
                            hostGame(settings, i); //Host a game
                        }
                    }
                }
            }
        }
        else
        {
            hostGame(settings, 0); //If there is no games currently being hosted, host one
        }

    function hostGame (settings, index)
    {
        var hostGame = new HostGame();

        hostGame.initialise(settings.userId, settings.userName, settings.boardSize, settings.gameMode, settings.gameNote);
        hostingGames.splice(index, 0, hostGame);
        server.updateGamesList(hostingGames);

        server.consoleLog('APP', settings.userName + ' is hosting a game. ID: ' + settings.userId);
    }
});



server.em.addListener('cancelHostGame', function(userId, userName)
{
    for (i in hostingGames)//For every game
    {
        if (hostingGames[i] != null) //If its not null
        {
            if (hostingGames[i].userId === userId) //If that user is hosting a game
            {
                hostingGames.splice(i, 1); //Remove their game
                server.updateGamesList(hostingGames);

                server.consoleLog('APP', userName + ' has stopped hosting a game. ID: ' + userId);
            }
        }
    }
});

I recommend to first apply the suggestions given in reply to your previous request for review here codereview.stackexchange.com/questions/161696/… - otherwise you might get a lot of duplicate advice. — le_m
– le_m, Commented Apr 26, 2017 at 14:43

Joseph · Accepted Answer · 2017-04-26 15:11:35Z

Same dude, same code, same review (except I noticed hostingGames is not an object like I assumed in the first answer).

Never use for-in on arrays. You risk looping through properties in addition to the array items. Use a regular for loop with indexes, or better, array methods.

And when splicing an array multiple times, it's best you do it in reverse. You risk skipping items when you do it forwards.

Also, it's best not to leave any holes in the array. From a developer that knows nothing about the context of your app, why are they empty? Especially when cancelHostGame splices cancelled games?

server.em.addListener('cancelHostGame', function(userId, userName){
  for(let i = hostingGames.length; i--;){
    let game = hostingGames[i];
    if(!game || game.userId !== userId) continue;
    hostingGames.splice(i, 1);
  }
});

Now the first part of your code is somewhat different from the first review. But the advice from the first review still applies.

However, you've introduced the concept of "slots". But in reality, all your code does is limiting the number of hosted games. Slots is just an abstract way of putting it. A much easier approach is to just check if the hosted array length is at the limit. If not at the limit, just push to the array.

server.em.addListener('hostGame', function(settings){

  const slotsMaxed = hostingGames.length === MAX_SLOTS;
  if(slotsMaxed) return;

  const userHasGame = hostingGames.map(g => g.userId).includes(settings.userId);
  if(userHasGame) return;

  const hostGame = new HostGame();
  hostGame.initialise(settings.userId, settings.userName, settings.boardSize, settings.gameMode, settings.gameNote);
  hostingGames.push(hostGame);
  server.updateGamesList(hostingGames);

  server.consoleLog('APP', `${settings.userName} is hosting a game. ID: ${settings.userId}`);
});

Stack Exchange Network

Array of hosted games

1 Answer 1

You must log in to answer this question.

Linked

Hot Network Questions

Array of hosted games

1 Answer 1

You must log in to answer this question.

Linked

Related

Hot Network Questions