2
\$\begingroup\$

I am new to Python and have written a function which calls an API and does some processing. Please review the code and explain what is the right and pythonic way.

def fetch_storedetails(api_link, zone_id, latitude, longitude):
    """

    :param api_link:
    :param zone_id:
    :param latitude:
    :param longitude:
    :return:
    """
    import requests
    store_details = requests.get(
        api_link + '/' + str(zone_id) + '/' + str(0) + '/' + str(latitude) + '/' + str(longitude),
        headers={'language': 'en'})
    print(store_details, store_details)
    print(store_details.json)
    s_details = store_details.json()
    if store_details.status_code == 400:
        return "'Sorry , we do not deliver to your selected address at this moment as it’s out of delivery reach."
    elif store_details.status_code == 404:
        return "Stores are not available in your location"
    elif store_details.status_code == 500:
        return "Internal server error"
    else:

        print(s_details)
        store_name = []
        store_address = []
        banner_image = []
        store_rating = []
        store_id = []

        for i in s_details['data']:
            store_name.append(i['businessName'])
            store_address.append(i['storeAddr'])
            banner_image.append(i['bannerLogos']['bannerimage'])
            store_rating.append(i['businessRating'])
            store_id.append(i['businessId'])

    return store_name, store_address, banner_image, store_rating, store_id
\$\endgroup\$
1
\$\begingroup\$
  • If you're going to drop in a comment block for function documentation, fill it out. Otherwise, delete it.
  • Generally you shouldn't import on the inside of a function. import at the top of a file.
  • Your constructed URL shouldn't use a series of +. You should be using the % formatting operator or str.format(). This will also avoid calling str(0).
  • None of your elses are necessary. You return before each of them, so you can simply continue writing the rest of the function afterward.
  • If I were you, I would simply return s_details['data']. Decomposing the payload into a series of lists isn't really useful, and if it is indeed useful for your application, it should be done in a separate function.
  • Returning an error string to indicate an error is a bad idea. You should raise an exception instead.
| improve this answer | |
\$\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.