-1
\$\begingroup\$

I'm wondering if the nested "try/except/else" structure (in the main function) is a good practice in Python. Right now, the structure is not that deep but I could continue on and on with additional conditions in the "else" part. Is there a better way to implement this ?

My code written in main.py:

import logging
import traceback
import flask
from google.cloud import firestore


class DocumentDoesNotExist(Exception):
    """
    Exception raised when there is no document corresponding to a given token in Firestore/collectionA nor
    Firestore/collectionB.
    """


class DocumentExistElsewhere(Exception):
    """
    Exception raised when there is no document corresponding to a given token in Firestore/collectionA but there is
    one in Firestore/collectionB.
    """


def get_patient_info(token):
    db = firestore.Client()
    document = db.collection(u'collectionA').document(token).get()

    if document.exists:
        return document.to_dict()
    else:
        document = db.collection(u'collectionB').document(token).get()
        if document.exists:
            raise DocumentExistElsewhere
        else:
            raise DocumentDoesNotExist


app = flask.Flask(__name__)


@app.route("/<request>")
def main(request: flask.Request):
    headers = {
        'Access-Control-Allow-Methods': 'OPTIONS, GET',
        'Access-Control-Allow-Headers': 'Content-Type',
        'Access-Control-Max-Age': '9600'
    }

    allowed_origins = [
        "http://localhost:5000"
    ]

    origin = flask.request.headers.get('Origin') or flask.request.headers.get('origin')
    if origin in allowed_origins:
        headers['Access-Control-Allow-Origin'] = origin

    if flask.request.method == 'OPTIONS':
        return '', 204, headers

    if flask.request.method == 'GET':
        try:
            request_json = flask.request.get_json(force=True, silent=True)  # Returns None if fails.
            token = request_json['token']
        except (TypeError, KeyError):
            logging.error(traceback.format_exc())
            return "Cannot retrieve token from request", 400, headers
        else:
            try:
                info = get_patient_info(token)
            except DocumentDoesNotExist:
                return "Token provided does not correspond to any document in Firestore/collectionA nor in " \
                       "collectionB.", 404, headers
            except DocumentExistElsewhere:
                return "Token provided corresponds to a document in Firestore/collectionB but none in " \
                        "Firestore/collectionA.", 404, headers
            else:
                return info, 200, headers


if __name__ == "__main__":
    app.run(debug=True)

You can run the Flask app like this python3 main.py and then testing with a curl curl -H "Origin: http://localhost:5000" -X GET -d '{"token":"06sr1"}' -i http://localhost:5000/test

Also, if you can see any problem with my implementation I'm open to any tips :)

\$\endgroup\$
5
  • 1
    \$\begingroup\$ I'm curious to hear how you expect the code to be used in the future. In particular I have 2 questions. Are you going to have many collections or will it be limited to these 2? What should a user do on a DocumentExistElsewhere response? \$\endgroup\$ Oct 27, 2021 at 10:55
  • \$\begingroup\$ @IvoMerchiers only 2 collections. But generally speaking it could expand. When DocumentExistElsewhere, I actually send a message to the front to display something. \$\endgroup\$
    – cuzureau
    Oct 27, 2021 at 11:53
  • \$\begingroup\$ Would you then expect the user to retry the query but for collectionB or is the access model more complicated? \$\endgroup\$ Oct 27, 2021 at 11:59
  • \$\begingroup\$ Every time the user do the query, it has to check collectionA before collectionB \$\endgroup\$
    – cuzureau
    Oct 27, 2021 at 12:48
  • 2
    \$\begingroup\$ Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. Thanks! \$\endgroup\$ Oct 27, 2021 at 13:57

1 Answer 1

3
\$\begingroup\$

The try ... except ... else ... construct works as follows.

try:
    print("Code protected by the try-statement")
except AnException:
    print("Exception handling code - not protected by try-statement")
else:
    print("No exception occurred code - not protected by try-statement")

print("Common code executed whether or not an exception occurred.")

Now let's add in a bit of your exception handling code...

try:
    print("Code protected by the try-statement")
except AnException:
    print("Exception handling code - not protected by try-statement")
    return
else:
    print("No exception occurred code - not protected by try-statement")

print("Common code executed whether or not an exception occurred ...")
print("except if an exception occurs, the code returns and won't get here!")

It should become obvious that the only way to get to the last two print() statements is if the print() statement in the else: clause is executed. If an exception occurs, the function exits via the return statement. Therefore, the else: clause is unnecessary.

try:
    print("Code protected by the try-statement")
except AnException:
    print("Exception handling code - not protected by try-statement")
    return

print("No exception occurred code - not protected by try-statement")
print("Common code executed whether or not an exception occurred ...")
print("except if an exception occurs, the code returns and won't get here!")

This means there is no need to have nested try ... except ... else ... statements. Simply omit the else: clauses.

        try:
            request_json = flask.request.get_json(force=True, silent=True)  # Returns None if fails.
            token = request_json['token']
        except (TypeError, KeyError):
            logging.error(traceback.format_exc())
            return "Cannot retrieve token from request", 400, headers

        # No else-clause here

        try:
            info = get_patient_info(token)
        except DocumentDoesNotExist:
            return "Token provided does not correspond to any document in Firestore/collectionA nor in " \
                   "collectionB.", 404, headers
        except DocumentExistElsewhere:
            return "Token provided corresponds to a document in Firestore/collectionB but none in " \
                   "Firestore/collectionA.", 404, headers

        # No else-clause here either

        return info, 200, headers
\$\endgroup\$

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.