0

I am developing a small web app to track points for a local board game group. The scoreboard will track a players total games played, total points earned as well as a breakdown of games played and points earned per faction available in the game.

I am currently hitting my head against a wall trying to create the node/express endpoint for /players/withFactions/, which will return aggregate player data as well as the per faction breakdown.

As you will see in the code attached, the logic works correctly and the data is correct right at the location of the first console.log. That data is simply returned and I am logging the return at the second console log, where it seems the data has changed. The mutated data seems to always take the form of the last returned element from the location of the first console.log. I hope this will make more sense upon review of the code.

I am sure my problem lies in how I am handling all of the async, but I cannot figure out the solution.

I have attacked this issue a multitude of different ways: callbacks, promises, async/await. All with the same outcome. The data seems to change on me right before being returned to the front end.

router.get('/withFactions', async (req, res) => {

    const { rows: players }  = await db.query('SELECT * FROM players');
    const { rows: factions }  = await db.query('SELECT * FROM factions');

    const playerResults = await Promise.all(players.map( async (player) => {
        await db.query('SELECT * FROM gameentry WHERE player_id = $1', [player.id]).then((result) => {
            const gameEntries = result.rows;
            const factionTotals = factions.map((faction) => {
                const factionEntries = gameEntries.filter(game => game.faction_id === faction.id)
                faction.totalPoints = factionEntries.reduce((acc, curr) => {
                    return acc + curr.points;
                }, 0)
                return faction;

            })
            player.factionTotals = factionTotals;
        })  
        player.factionTotals.forEach(element => console.log(element.totalPoints)) 

// total scores are recording correctly here   4,0,0   5,0,3   0,5,0 (from DB data)

        return await player;
    }))

    await playerResults.forEach(player => player.factionTotals.forEach(element => console.log(element.totalPoints))); 

// total scores are wrong here. Each set is 0,5,0. Seems to just replicate the last set from within the promise

    res.send(await playerResults);
})

NOTE: the ONLY data that is incorrect is player.factionTotals the rest of the player data remains accurate.

I am sure there are a few more awaits than necessary in there. I have been looking at this problem for too long. I hope I can get some assistance on this one.

6
  • by the way, to make the code more readable, you don't need await in return await player or await playerResults.forEach or res.send(await playerResults); - since none of these are promises Commented Aug 11, 2019 at 2:27
  • A minimal example that could be run independently (e.g. with just sample array promises instead of the DB queries) would be helpful. Commented Aug 11, 2019 at 2:29
  • this bit of code factionTotals = factions.map updates const { rows: factions } every time, so of course the final result will be the result of the final iteration of players.map ... try const factionTotals = factions.slice().map((faction) => { Commented Aug 11, 2019 at 2:36
  • 1
    @JaromandaX: That seems like the issues, might as well post that as an answer. Which goes to show that modifying state in map is usually not a good idea... Commented Aug 11, 2019 at 2:37
  • @JaromandaX: Slicing probably won't help, that's just a shallow copy, the faction objects will still be the same. Commented Aug 11, 2019 at 2:38

1 Answer 1

1

As identified by Jaromanda X the issue is that you modify the factions for every player, so only the last modification will "stick around".

In general i would recommend to not have side effects when using map and only return new objects or unmodified objects. To that end you should be able to modify your code to something like this:

router.get('/withFactions', async (req, res) =>
{
    const { rows: players } = await db.query('SELECT * FROM players');
    const { rows: factions } = await db.query('SELECT * FROM factions');

    const playerResults = await Promise.all(players.map(async player =>
    {
        const { rows: gameEntries } = await db.query('SELECT * FROM gameentry WHERE player_id = $1', [player.id]);

        const factionTotals = factions.map(faction =>
        {
            const factionEntries = gameEntries.filter(game => game.faction_id === faction.id)
            var totalPoints = factionEntries.reduce((acc, curr) => acc + curr.points, 0);

            return { ...faction, totalPoints }; // Copies faction properties instead of modifying
        })

        const extendedPlayer = { ...player, factionTotals };

        extendedPlayer.factionTotals.forEach(element => console.log(element.totalPoints))

        return extendedPlayer;
    }))

    playerResults.forEach(player => player.factionTotals.forEach(element => console.log(element.totalPoints)));

    res.send(playerResults);
})

Using the spread (...) copies the properties, so the original factions list is the same for every player iteration. The second spread for creating extendedPlayer is probably not strictly necessary to prevent your issue, but keeps the outer map pure as well.

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

3 Comments

that's much nicer than slice :p
@JaromandaX: Thanks, i hope you don't mind me having answered this. (I assumed you would have posted an answer by now, so i went ahead.)
Thanks so much for the help! That did it. Really appreciate the backup!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.