2
\$\begingroup\$

I am building my first REST API using data stored in my MongoDB Atlas Database. I tried to read the REST API conventions with the versioning, using plural nouns for resources and sending the right error codes. I wonder if the following code correctly follows Good Practices for REST APIs and if there is anything that can be written better or more neatly.

connection_str = config.connection_str
db_name = config.db_name

app = Flask(__name__)
cors = CORS(app)
client = MongoClient(connection_str)
db = client.get_database(db_name)

@app.route("/api/v1/players", methods = ['GET'])
def get_all_players():
    # TODO: Implement pagination
    """
    returns a list of players
    """
    try:
        players = db.players.find()
        output = []
        for player in players:
            output.append({
                'name': player['name'],
                'position': player['position'],
                'nationality': player['nation'],
                'position': player['position'],
                'foot': player['foot'],
                'age': utils.date_to_age(player['birth_date']),
                'profile_img': player['profile_img'],
                'abilities': player['abilities']
            })
        return jsonify({'result': output})

    except:
        return jsonify({
            'result': 'failure',
            "error": 400,
            "message": 'Bad Request'}), 400

@app.route("/api/v1/players/<name>", methods = ['GET'])
def get_player(name):
    """
    returns an object of a player given a name
    """
    try:
        player = db.players.find_one({'name': name})
        output = {
            'name': player['name'],
            'position': player['position'],
            'nationality': player['nation'],
            'position': player['position'],
            'foot': player['foot'],
            'age': utils.date_to_age(player['birth_date']),
            'profile_img': player['profile_img'],
            'abilities': player['abilities']
        }
        return jsonify({'result': output})

    except:
        return jsonify({
            'result': 'failure', 
            "error": 400, 
            "message": "Bad Request (Double check player's name)"}), 400

if __name__ == "__main__":
    # app.run(ssl_context='adhoc')
    app.run()

The API doesn't do that much right now. It just reads data in the DB, but I want to see if any style can be fixed at this early stage.

\$\endgroup\$
1
\$\begingroup\$

In general, I think the code looks pretty good. I would suggest two changes:

Dry

In an effort to keep the code DRY, I suggest constructing the player dict in a function for that purpose like:

def player_to_dict(player):
    return {
        'name': player['name'],
        'position': player['position'],
        'nationality': player['nation'],
        'position': player['position'],
        'foot': player['foot'],
        'age': utils.date_to_age(player['birth_date']),
        'profile_img': player['profile_img'],
        'abilities': player['abilities']
    }

This takes out quite a bit of code from:

def get_player(name):
    try:
        player = db.players.find_one({'name': name})
        return jsonify({'result': player_to_dict(player)})
    ....

I like comprehensions:

Using a list comprehension and the new player_to_dict(), you can remove the explicit for loop and instead make get_all_players much smaller and looking like quite similar to get_player.

def get_all_players():
    try:
        players = db.players.find()
        return jsonify({'result': [player_to_dict(player) for player in players]})
    ....
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot!!! \$\endgroup\$ – Dawn17 Jun 30 at 2:38
  • \$\begingroup\$ Would it be better to put player_to_dict inside a utils file as I did for the birthdate to age conversion, or shall I just put it with the api? \$\endgroup\$ – Dawn17 Jun 30 at 2:41
  • 1
    \$\begingroup\$ Actually a fairly tough call. I tend to like to keep things as local as possible. If there is any chance of the code being used outside the file, then utils is great. But if it is just a helper for this file, then leave it here. \$\endgroup\$ – Stephen Rauch Jun 30 at 2:43

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.