HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Using the Rotten Tomatoes API

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theusingrottenapitomatoes

Problem

This is a Python module I made to search the Rotten Tomatoes movie API. It caches the search results in a SQLite database.

What improvements can I make to the cache system? Is there a better way to store the results of the API search?

Is there a better way to communicate with the API?

```
import urllib2
import json
import sqlite3
import time

# sqlite database file
cache_database = "movies.db"
# how many seconds before the entry expires
cache_expiration = 60*60 # one hour

class Cache:
def get_conn(self):
"""
connect with the sqlite database
"""
conn = sqlite3.connect(cache_database)
c = conn.cursor()
c.execute(""" CREATE TABLE IF NOT EXISTS movies(
search_query TEXT PRIMARY KEY,
page_number INT,
timestamp INTEGER,
search_results BLOB); """)
return conn

def get(self, search_query, page_number):
"""
get the search results from the database
"""
with self.get_conn() as conn:
c = conn.cursor()
query = """SELECT search_results FROM movies WHERE
search_query = \"{search_query}\" AND
page_number = {page_number} AND
(strftime('%s', 'now') - timestamp) < {cache_expiration}"""

query = query.format(search_query = search_query,
page_number = page_number,
cache_expiration = cache_expiration)
results = ""
for a in c.execute(query):
results = a
return results

def put(self, search_query, page_number, search_results):
"""
put the results into the database
"""
timestamp = int( time.time() )
with self.get_conn() as conn:
c = conn.cursor()
insert = """ INSERT OR REPLACE INTO movies

Solution

It's a pretty nice script! (You should open-source it, I would use it.)

Prepared statements

You should use prepared statements in your queries. It's not only easier to write, it's also more efficient. For example:

def get(self, search_query, page_number):
    with self.get_conn() as conn:
        c = conn.cursor()
        query = """SELECT search_results FROM movies
                   WHERE search_query = ?
                   AND page_number = ?
                   AND strftime('%s', 'now') - timestamp < ?; """

        c.execute(query, (search_query, page_number, cache_expiration))
        return c.fetchone()


Notice that you don't need to worry about quoting anymore.

You can do the same thing to your Cache.put method too:

def put(self, search_query, page_number, search_results):
    timestamp = int(time.time())
    with self.get_conn() as conn:
        c = conn.cursor()
        insert = """INSERT OR REPLACE INTO movies
                    (search_query, page_number, timestamp, search_results)
                    VALUES (?, ?, ?, ?); """

        c.execute(insert, (search_query, page_number, timestamp, search_results,))
        conn.commit()


Statements can be simplified

Instead of this:

results = "" 
for a in c.execute(query):
    results = a
return results


It would be simpler and cleaner like this:

c.execute(query)
return c.fetchone()


Unless, you really want to iterate over multiple results and return only the last one, as it was in your original code.

Python classes

Modern classes should extend object, for example:

class Cache(object):
    ...

class Movie(object):
    ...


Coding style

You can simplify this:

if result != "":
    ...


like this:

if result:
    ...


And it's good to follow PEP8, the official Python style guide.

Code Snippets

def get(self, search_query, page_number):
    with self.get_conn() as conn:
        c = conn.cursor()
        query = """SELECT search_results FROM movies
                   WHERE search_query = ?
                   AND page_number = ?
                   AND strftime('%s', 'now') - timestamp < ?; """

        c.execute(query, (search_query, page_number, cache_expiration))
        return c.fetchone()
def put(self, search_query, page_number, search_results):
    timestamp = int(time.time())
    with self.get_conn() as conn:
        c = conn.cursor()
        insert = """INSERT OR REPLACE INTO movies
                    (search_query, page_number, timestamp, search_results)
                    VALUES (?, ?, ?, ?); """

        c.execute(insert, (search_query, page_number, timestamp, search_results,))
        conn.commit()
results = "" 
for a in c.execute(query):
    results = a
return results
c.execute(query)
return c.fetchone()
class Cache(object):
    ...

class Movie(object):
    ...

Context

StackExchange Code Review Q#49785, answer score: 4

Revisions (0)

No revisions yet.