4
\$\begingroup\$

I'd like some feedback on the readability, style, and potential problems or issues.

foursquare.py

import time

from api_keys import CLIENT_ID
from api_keys import CLIENT_SECRET
from api_keys import CATEGORY_ID

from business import Business
from business import make_request


def search(lat, lng, distance):
    """
    Searches the Foursquare API (Max Limit = 50)

    :param lat: Latitude of the request
    :param long: Longitude of the request
    :param distance: Distance to search (meters)
    :returns: List of retrieved venues
    """

    url = 'https://api.foursquare.com/v2/venues/explore?ll=%s,%s&intent=browse&radius=%s&limit=50&categoryId=%s&client_id=%s&client_secret=%s&v=%s' % (lat, lng, distance, CATEGORY_ID, CLIENT_ID, CLIENT_SECRET, time.strftime("%Y%m%d"))
    venue_list = []

    try:
        data = make_request(url)

        for item in data['response']['groups'][0]['items']:
            venue = item['venue']
            venue_list.append(Business(venue['name'],
                                       venue['location']['address'],
                                       venue['rating'],
                                       venue['ratingSignals'],
                                       venue['stats']['checkinsCount']))
    except Exception, e:
        print e

    return venue_list

business.py

import urllib2
import json


class Business:
    def __init__(self, name, address, rating, rating_count, checkin_count):
        self.name = name
        self.address = address
        self.rating = rating
        self.rating_count = rating_count
        self.checkin_count = checkin_count


def make_request(url):
    """
    Makes a new HTTP request to the given URL

    :param url: The URL to request
    :returns: JSON response
    """

    req = urllib2.Request(url)
    response = urllib2.urlopen(req)
    data = json.loads(response.read())
    response.close()

    return data
\$\endgroup\$

2 Answers 2

8
\$\begingroup\$

Named tuples

Business is nothing short of a tuple where each attribute is named. Rather than going through the verbosity of writing everything out, you can just make it a namedtuple:

Business = namedtuple('Business', 'name address rating rating_count checkin_count')

Managing resources

For make_request, the preferred way to handle resources in Python is to use a context manager. For urlopen, we have contextlib.closing:

def make_request(url):
    with closing(urlopen(url)) as response:
        return json.loads(response.read())

Do you need to wrap it in Request? Not sure that gives you anything.

String formatting

Prefer the format() syntax to the % syntax. That is:

'Hello, {}'.format(name)

is better than:

'Hello, %s' % name
\$\endgroup\$
2
  • \$\begingroup\$ Good suggestion with the named tuples. Definitely improves the readability! I'll explore using the context manager as well. Is there a reason format is preferred? Just curious if it's personal preference or not. Thanks for your help! \$\endgroup\$
    – leerob
    Dec 10, 2015 at 19:11
  • 1
    \$\begingroup\$ @leerob See string formatting \$\endgroup\$
    – Barry
    Dec 10, 2015 at 19:28
6
\$\begingroup\$

In addition to answer by Barry, I would like to state that your code has nice structure, good comments and reads easily. There are however a few minor points I would make:

  • Join the imports from the same module – In general you shouldn't join multiple imports at the same line, but when getting things from the same module the recommendation is to join it like in: from api_keys import CLIENT_ID, CLIENT_SECRET, CATEGORY_ID

  • Extract the url pattern into a constant – Most of the search url is constant, and could be moved to the top. Building the url has then a cleaner better focus on the changing parts:

    url = SEARCH_URL.format(lat, lng, distance,
                            CATEGORY_ID, CLIENT_ID, CLIENT_SECRET,
                            time.strftime("%Y%m%d"))
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.