patternpythonMinor
Using the Rotten Tomatoes API
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
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:
Notice that you don't need to worry about quoting anymore.
You can do the same thing to your
Statements can be simplified
Instead of this:
It would be simpler and cleaner like this:
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
Coding style
You can simplify this:
like this:
And it's good to follow PEP8, the official Python style guide.
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 resultsIt 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 resultsc.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.