3
\$\begingroup\$

Approach

All data is submitted into my DynamoDB from another Lambda > API Integration function whereas the lastUpdated row gets inserted as null and then the function below basically polls my database every 1 minute looking for new rows that have a null value & performs actions on them until the lastUpdated can then be updated (once an action is performed elsewhere)

I have the following Node.JS (runtime v8.10) executing on AWS Lambda:

const AWS = require("aws-sdk");
const game = require('game-api');
const uuid = require("uuid");

AWS.config.update({
  region: "us-east-1"
});

exports.handler = async (event, context) => {

    //set db
    var documentClient = new AWS.DynamoDB.DocumentClient();

    //find matches
    var params = {
        TableName: 'matches',
        FilterExpression:'updated_at = :updated_at',
        ExpressionAttributeValues: {
            ":updated_at": 0,
        }
    };
    var rows = await documentClient.scan(params).promise();

    //game object
    let gameAPI = new game(
        [
            "admin@game.com",
            "password"
        ]
    );

    await gameAPI.login();

    for (let match of rows.Items) {

        var player_params = {
            TableName: 'players',
            Key: { "match_id": match.id }
        };

        let player_row = await documentClient.get(player_params).promise();

        //grab stats and compare
        var stats = await gameAPI.getStatsBR(
            player_row.Item.player_1_name,
            player_row.Item.player_1_network
        );
        var new_data = compareModiified(
            match.match_type,
            player_row.Item.player_1_last_updated,
            stats
        );

        if(new_data === true) {

                //we have new data
                let kills;
                let matches;
                let kills_completed;
                let matches_completed;
                let modified;

                switch(match.match_type) {
                    case 'myself':
                        kills = stats.group.solo.kills;
                        kills_completed = (kills - player_row.Item.player_1_kills);
                        matches = stats.group.solo.matches;
                        matches_completed = (matches - player_row.Item.player_1_matches);
                        modified = stats.group.solo.lastModified;
                        break;
                    case 'solo':
                        kills = stats.group.duo.kills;
                        kills_completed = (kills - player_row.Item.player_1_kills);
                        matches = stats.group.duo.matches;
                        matches_completed = (matches - player_row.Item.player_1_matches);
                        modified = stats.group.duo.lastModified;
                        break;
                    case 'duo':
                        kills = stats.group.squad.kills;
                        kills_completed = (kills - player_row.Item.player_1_kills);
                        matches = stats.group.squad.matches;
                        matches_completed = (matches - player_row.Item.player_1_matches);
                        modified = stats.group.squad.lastModified;
                        break;
                }

                var update_params = {
                    TableName:"matches",
                    Key: { "id": match.id },
                    UpdateExpression: "SET #status = :status, updated_at = :modified",
                    ExpressionAttributeNames:{
                        "#status":"status"
                    },
                    ExpressionAttributeValues:{
                        ":status": 1,
                        ":modified": modified
                    }
                };
                await documentClient.update(update_params).promise();

                var report_params = {
                    Item: {
                        'match_id': match.id,
                        'kills': kills_completed,
                        'matches': matches_completed,
                        'completed_at': new Date().getTime()
                    },
                    TableName : 'reports'
                };
                await documentClient.put(report_params).promise();

            } else {

                //we don't have new data.
                console.log("We don't have new data, let's not do anything..");

            }

    }

    return {
        statusCode: 200
    };

};

function compareModiified(match_type, db_modifiied, stats) {
    var stats_modified;
    switch(match_type) {
        case 'myself':
            stats_modified = stats.group.solo.lastModified;
            break;
        case 'solo':
            stats_modified = stats.group.duo.lastModified;
            break;
        case 'duo':
            stats_modified = stats.group.squad.lastModified;
            break;
    }
    return (stats_modified > db_modifiied);
}
\$\endgroup\$
  • 3
    \$\begingroup\$ The calculation for kills_completed and matches_completed could be done after the switch statement since they're all identical. Then you'd have 4 less lines of code and the same functionality. \$\endgroup\$ – Shelby115 Apr 3 at 14:42
3
+100
\$\begingroup\$

Welcome to Code Review! Hopefully this experience will be useful and positive for you.

Your statement about polling

and then the function below basically polls my database every 1 minute looking for new rows that have a null value & performs actions on them until the lastUpdated can then be updated (once an action is performed elsewhere)

I haven't used AWS and DynamoDB but perhaps you could use a trigger or some other hook to receive a notification instead of polling each minute. With this approach, the server doesn’t waste resources checking for an update but rather processes incoming data when necessary. Perhaps this page would be useful.

General feedback

Overall the handler function looks a bit too long. The suggestions below should help you reduce the length of it, though if it is still too long then it may be advisable to break it up into smaller atomic functions that handle a single part of getting data.

The variable update_params could be moved outside the handler function without the id property of the Key property and :modified property of ExpressionAttributeValues set - those can be set when needed.

const update_params = {
    TableName:"matches",
    Key: {},
    UpdateExpression: "SET #status = :status, updated_at = :modified",
    ExpressionAttributeNames:{
        "#status":"status"
    },
    ExpressionAttributeValues:{
        ":status": 1
    }
};

Then when before passing it to the call to documentClient.update(update_params).promise();:

update_params.Key.id = match.id;
update_params.ExpressionAttributeValues[":modified"] = modified;

Your code could make more use of const - especially for values that are never reassigned - e.g. documentClient, params, rows, gameAPI, player_params, etc. Some developers believe it is wise to default to using const and then if the need to re-assign presents itself, switch to using let.'

I would also question whether you intentionally used var instead of let within the for loop...


Let's look at that function compareModiified():

function compareModiified(match_type, db_modifiied, stats) {
    var stats_modified;
    switch(match_type) {
        case 'myself':
            stats_modified = stats.group.solo.lastModified;
            break;
        case 'solo':
            stats_modified = stats.group.duo.lastModified;
            break;
        case 'duo':
            stats_modified = stats.group.squad.lastModified;
            break;
    }
    return (stats_modified > db_modifiied);
}

Did you intentionally spell the function name as compareModiified and the second parameter as db_modifiied, or are those double i's typos?

This could be simplified using a mapping of match_type to the property of stats.group to access:

const matchTypeGroupMapping = {
    'myself' => 'solo',
    'solo' => 'duo', 
    'duo' => 'squad'
};

A Map could also be used instead of a plain old JavaScript Object - useful if the keys were not serializable.

That mapping could be used to simplify the function:

function compareModiified(match_type, db_modifiied, stats) {
    if (match_type in matchTypeGroupMapping) {
         const mappedKey = matchTypeGroupMapping[match_type];
         return stats.group[mappedKey].lastModified > db_modifiied;
    }
    //do we need to handle other values of match_type?
}

As the comment at the end alludes to: what should happen for other values of match_type?


That mapping define above could also be used to simplify the cases within the switch statement of the for loop inside the handler function. And Shelby115's comment is correct - the assignment statements in all three cases of the switch statement are identical and could be moved out of the switch statement. But the code below can be used to replace the switch statement entirely.

if (new_data === true && match.match_type in matchTypeGroupMapping) {
    const mappedKey = matchTypeGroupMapping[match.match_type];
    const kills = stats.group[mappedKey].kills;
    const matches = stats.group[mappedKey].matches;

    update_params.ExpressionAttributeValues[":modified"] == stats.group[mappedKey].lastModified;

    await documentClient.update(update_params).promise();

    const report_params = {
        Item: {
            'match_id': match.id,
            'kills': (kills - player_row.Item.player_1_kills),
            'matches': (matches - player_row.Item.player_1_matches),
            'completed_at': new Date().getTime()
        },
        TableName : 'reports'
    };
    await documentClient.put(report_params).promise();
}

Notice how above the variables kills_completed and matches_completed have been eliminated, but if you want to keep them for clarity you could.

\$\endgroup\$
  • \$\begingroup\$ Absolutely fantastic answer! it's exactly what I was looking for so I couldn't thank you enough for this \$\endgroup\$ – Curtis Apr 14 at 16:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.