# Making datastore inserts with Google App Engine and WTForms

I have a class for making datastore inserts with Google App Engine and WTForms. Now I want to refactor it into smaller units but I'm not sure how to do it. Can you help me find what needs to be done? The code consists of code for taking a form (AdForm) and its parameters to make a datastore insert. It also checks for inapproriate content in a way I'd like to improve.

class AdLister(BaseRequestHandler,
csrf_protect = False

def post(self):

if users.get_current_user():

if self.current_user is not None:
try:
except Exception, e:
logging.info('exception %s' % str(e))

if self.request.get('type'):
if self.request.get('rooms'):
if self.request.get('size'):
if self.request.get('regdate'):
if self.request.get('mileage'):

if form.validate():
title = to_unicode_or_bust(form.title.data)
#unicode(form.title.data, 'utf-8')
name = to_unicode_or_bust(form.name.data)  #, 'utf-8')
try:
if form.phonenumber.data:
except:
pass

text = to_unicode_or_bust(form.text.data)  # , 'utf8')
titletest = to_unicode_or_bust(form.title.data)

if 'penis' in text:
self.response.out.write('REMOVED')
return

if 'Black Money' in text:
self.response.out.write('REMOVED')
return

if 'black money' in text:
self.response.out.write('REMOVED')
return

if 'BLACK MONEY' in titletest:
self.response.out.write('REMOVED')
return

if 'BLACK DOLARS' in titletest:
self.response.out.write('REMOVED')
return

if 'Penis' in text:
self.response.out.write('REMOVED')
return

if 'penis' in title:
self.response.out.write('REMOVED')
return

if 'Penis' in title:
self.response.out.write('REMOVED')
return

if 'escort' in text:
self.response.out.write('REMOVED')
return
if 'escorts' in text:
self.response.out.write('REMOVED')
return
if 'escort' in title:
self.response.out.write('REMOVED')
return
if 'escorts' in title:
self.response.out.write('REMOVED')
return

ad.price = form.price.data.replace(' ', '').replace(',00',
'').replace('.00', '')
try:
if form.price.data:
ad.integer_price = form.price.data.replace(' ', ''
).replace(',00', '').replace('.00', '')
except:
pass
self.session['url'] = self.request.host
else:

'user': self.current_user,
'session': self.auth.get_user_by_session(),
'request': self.request,
'form': form,
'name': to_unicode_or_bust(form.name.data)  #.encode('utf-8')
})
return
if self.request.get('currency'):
if self.request.get('cg'):
if self.request.get('company_ad') == '1':

try:
image_url = images.get_serving_url(str(upload.key()), size=640)
img.put()
except Exception, e:
logging.error('There was an exception:%s' % str(e))
pass
if self.request.get('area'):
city = \
montaomodel.City.get_by_id(long(self.request.get('area'
)))
region = montaomodel.Region.get(city.region.key())

if self.current_user:

image_url = None
if image:
if image.primary_image:
try:
image_url = \
images.get_serving_url(str(image.primary_image.key()),
size=640)
except Exception, e:
image_url = '/images/' + str(image.key().id()) \
+ '_small.jpg'
else:
image_url = '/images/' + str(image.key().id()) \
+ '_small.jpg'
imv = []
for i in ad.matched_images:
if i.primary_image:
try:
i1 = \
images.get_serving_url(str(i.primary_image.key()))
imv.append(i1)
except Exception, e:
i1 = '/images/' + str(image.key().id()) \
+ '_small.jpg'
imv.append(i1)

if ad.price:  # and doesn't contain separators
try:
price = \
except Exception, e:
else:

self.render('preview.html', {
'user': self.current_user,
'session': self.auth.get_user_by_session(),
'request': self.request,
'image_url': image_url,
'imv': imv,
'len': len(imv),
'price': price,
})


You've got a massive repetition:

if 'penis' in text:
self.response.out.write('REMOVED')
return

if 'Black Money' in text:
self.response.out.write('REMOVED')
return

if 'black money' in text:
self.response.out.write('REMOVED')
return

if 'BLACK MONEY' in titletest:
self.response.out.write('REMOVED')
return

if 'BLACK DOLARS' in titletest:
self.response.out.write('REMOVED')
return

if 'Penis' in text:
self.response.out.write('REMOVED')
return

if 'penis' in title:
self.response.out.write('REMOVED')
return

if 'Penis' in title:
self.response.out.write('REMOVED')
return

if 'escort' in text:
self.response.out.write('REMOVED')
return
if 'escorts' in text:
self.response.out.write('REMOVED')
return
if 'escort' in title:
self.response.out.write('REMOVED')
return
if 'escorts' in title:
self.response.out.write('REMOVED')
return


You can use a list comprehension to shorten this very much:

BAD_WORDS = ['penis', 'black money', 'escort']

if any([w in text or w.upper() in text or w.capitalize() in text
for w in BAD_WORDS]):
self.response.out.write('REMOVED')
return


Never use bare except:, always tell the expected exception.

• Thank you. I'm still learning python and the way you show is much more concise and better. – Niklas May 11 '15 at 13:12
• any[w in text doesn't look right to me. Shouldn't there be parentheses? – Kevin May 11 '15 at 13:24
• Please take a bit more care over the code snippets, or this happens: stackoverflow.com/q/30168806/3001761 – jonrsharpe May 11 '15 at 13:24
• Wouldn't w in text.lower() be more concise and cover all the cases? – Morwenn May 11 '15 at 15:55

# Style

You have commented out bits of code. Use version control and don't keep that stuff around (unless you have a very good reason; I don't see one here).

In some cases the formatting is off. The general advice is to follow PEP8, that's very easy to accomplish if you run the corresponding tools, i.e. pep8 or so.

# Code

First of all: You should split your functionality into smaller chunks. At the moment you have one massive method which does way too many things.

I'd suggest something along the lines of one method for parsing input, initialising the Ad object, validating input, importing uploaded files, rendering output, etc. That way you can later actually see what the control flow is, because at the moment you have returns mixed into your logic and it's quite confusing to follow.

For filtering certain keywords I'd actually recommend constructing a regular expression for all words. That way you can use the features of the engine to specify patterns more easily than with that loop with lower etc. It might be more efficient as well since you can precompile the regex and run it only once on the whole text. Then again, if you're still learning maybe just keep what you currently have and take a look at it later. See also e.g. this SO post.

import re
BAD_WORDS = ['penis', 'black money', 'escort']