Gfrktyl

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

• 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.