Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

Posted by Jamison Habermann
Jamison Habermann
11,691 Points

Coding Design question. Return values up a method chain, or call a function at the end of chain instead.

I'm creating a game in node.js using socket.io, and I thought I had a good system down with handling returning values from my server -> game instance -> round instance objects, but over time my own code is just getting more confusing. An example:

socket.on('join game', gameID => {
    const full = gameServer.joinGame(id, socket.id)
    if (full) {
        io.in(socket.currentGame).emit('ready check');
    }
}

gameServer.joinGame(id, userId){
    const game = this.getGame(id) // get the instance of the game
    const full = game.addPlayer(userId, username);
    if (full){
        return full
    }
}

game.addPlayer(userId) {
    this.players.push(userId)
    if (this.players.length === this.minimumPlayersRequired) {
        return true; // this is the "full" flag im sending back up the chain
    }
}

What I'm wondering is if this is the "proper" way to do things. In my case, the 3 steps are separated into different modules. I kept telling myself it is important to keep the logic separated but now I'm just thinking I ditch all the returns and simply call the socket.io emit at the very last step?

game.addPlayer(userId) {
    this.players.push(userId)
    if (this.players.length === this.minimumPlayersRequired) {
        io.in(gameid).emit('ready check');
    }
}

The problem with this is that I am potentially creating a circular dependency which is an indicator if bad design.. Or am I just overthinking this..??

1 Answer

Steven Parker
Steven Parker
231,269 Points
Steven Parker
Steven Parker
231,269 Points

I would need to know more about the overall program to be sure, but at first glance the original code seems to adhere better to the concept of "separation of concerns". It makes sense for the I/O to be handled in a separate place from a function that is updating the internal data.

But another "best practice" would be to always return an explicit value instead of relying on "undefined" to indicate a "false" condition:

game.addPlayer(userId) {
    this.players.push(userId)
    return this.players.length === this.minimumPlayersRequired;  // always return the result
}

Similarly, you can eliminate the "if" in "joinGame" and always "return full;".

Jamison Habermann
Jamison Habermann
11,691 Points

Thank you so much for answering my question! I'll implement your suggestions.