0
\$\begingroup\$

As a beginner, I'd like to see if anyone can suggest any better way of coding the below, as I'm duplicating code and nesting identical if statements as part of the following problem.

I'm grabbing data from a REST API, which is returned in JSON format. However, the query may fail. One cause of failure is that the authorization token obtained earlier has expired. In this case, I want to get a new token, and retry the query. Then I need to reprocess the response, which may still be a failure (but, this time, not because of an expired token).

So, I've written this, but I'm thinking there might be a more elegant way to write it to avoid code repeats:

# Firstly, I get the data, which returns a dict from a JSON response
response = get_data(authorization)
if response["responseStatus"] == "FAILURE":
    # I believe I need to check this in a separate if statement
    # because even though there's always a responseStatus key
    # it will only contain an errors key if the former equals FAILURE
    if response["errors"][0]["type"] == 'INVALID_SESSION_ID':
        print("Session expired, initiating new session.")
        # I call the function that grabs a new Authorization token
        authorization = get_authorization(username, password)
        # Then I call again the function that gets the data
        response = get_data(authorization)
        # And now I need to look again for a possible failure
        if response["responseStatus"] == "FAILURE":
            print(response["errors"])
    else:
        print(response["errors"])
# Here I couldn't do a simple else because I need to process
# success for either calls of get_data() above (before or
# inside the if block)
if response["responseStatus"] == "SUCCESS":
    process_data()
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – Toby Speight Aug 20 '18 at 12:36
0
\$\begingroup\$

EDIT: Answer updated to use a while-loop rather than recursion, as noted in the comments. I also recently learned about the max recursion depth in Python being a good argument against recursion in the first place.

One way you could solve this is by separating the error handling logic into its own function, and then repeatedly calling a connection function:

# Handles an error in the given response. Returns True if we should try connecting again
def handle_error(response):
    if response["errors"][0]["type"] == 'INVALID_SESSION_ID':
        print("Session expired, initiating new session.")
        authorization = get_authorization(username, password)
        return True
    return False

def connect():
    attempt_connection = True
    while attempt_connection:
        response = get_data(authorization)
        if response["responseStatus"] == "SUCCESS":
            attempt_connection = False
            process_data()
        elif response["responseStatus"] == "FAILURE":
            attempt_connection = handle_error(response)
            if not attempt_connection:
                print(response["errors"])

handle_error could, of course, be split up into more functions, one for each potential error. Depending on the response format, you may also want to check for each key in response before accessing it.

You could also specify retry as a number of times to reconnect before giving up, i.e if you were attempting to connect over multiple proxies. Then you would just decrement the counter after each failure, and print the error when the counter hits zero.

\$\endgroup\$
  • 1
    \$\begingroup\$ Erm... I would not opt for recursion here. A while-loop is cleaner and most likely faster. \$\endgroup\$ – Daniel Aug 21 '18 at 14:55

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.