2
\$\begingroup\$

Background is this question and to create something like Craigslist or olx.com i.e. classifieds advertisement webapp / site and I've basically solved the problem and had a mess of unstructured code that built other code and generated SQL and other stuff and now I've first rewritten it from Java which got the source down from 30 MB to about ½ MB and fixed tons of old bugs to now in Python (GAE) and the Python file to handle Brazil looks basically like this

from __future__ import with_statement
# coding=utf-8
import os
from google.appengine.api.users import is_current_user_admin, UserNotFoundError
import time
import cgi
import geo.geotypes
import main
from main import Ad
import captcha
import facebookconf
from google.appengine import api
from google.appengine.ext import webapp,  search
from google.appengine.ext.webapp import template
from google.appengine.runtime import DeadlineExceededError
from google.appengine.ext.webapp.util import run_wsgi_app
from google.appengine.ext import db
from google.appengine.api import users
from google.appengine.ext import blobstore
from google.appengine.ext.blobstore import BlobInfo
from google.appengine.api import images
from google.appengine.ext.db import djangoforms
from django import forms
from django.core.exceptions import ValidationError
from paginator import Paginator, InvalidPage, EmptyPage
from django.utils import translation
from datetime import datetime, timedelta
os.environ['DJANGO_SETTINGS_MODULE'] = 'conf.settings'
from django.conf import settings
from django.template import RequestContext
from util import I18NHandler, FacebookBaseHandler
import util
from google.appengine.api import urlfetch, taskqueue
from django.template.defaultfilters import register
from django.utils import simplejson as json
from functools import wraps
from google.appengine.api import urlfetch, taskqueue
from google.appengine.ext import db, webapp
from google.appengine.ext.webapp import util, template
from google.appengine.runtime import DeadlineExceededError
from random import randrange
import Cookie
import base64
import cgi
import conf
import datetime
import hashlib
import hmac
import logging
import time
import traceback
import urllib
import twitter_oauth_handler
from twitter_oauth_handler import OAuthClient
from geo.geomodel import GeoModel
from google.appengine.api import images


from django.utils.translation import gettext_lazy as _
webapp.template.register_template_library('common.templatefilters')
logo = 'market'

class Handler(FacebookBaseHandler,I18NHandler):
    def get(self, region='São Paulo', city='Grande São Paulo', category='For sale', cursor=None, limit=60, PAGESIZE = 50, twittername = None):
        lon = -46.38
        lat = -23.33

        region = region.replace("_"," ").title()
    if region is city:
        city = None        
    else: 
        city = city.replace("_"," ").title()
    category = category.replace("_"," ")
        page = int(self.request.GET.get('page', '1'))
    timeline = datetime.datetime.now () - timedelta (days = limit)
        m=int(self.request.get('r')) if self.request.get('r') else 804670
        q = self.request.get('q').encode("utf-8")
        location_map = {
      1: {'name': 'São Paulo', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      2: {'name': 'Rio De Janeiro', 'lat': -22.90, 'long': -43.21, 'radius': 294200},
      3: {'name': 'Acre', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      4: {'name': 'Alagoas', 'lat': -22.90, 'long': -43.21, 'radius': 294200},
      5: {'name': 'Amazonas', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      6: {'name': 'Amapá', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      7: {'name': 'Bahia', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      8: {'name': 'Ceará', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      9: {'name': 'Distrito Federal', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      10: {'name': 'Espírito Santo', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      11: {'name': 'Goiás', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      12: {'name': 'Mato Grosso', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      13: {'name': 'Maranhão', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      14: {'name': 'Minas Gerais', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      15: {'name': 'Pará', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      16: {'name': 'Paraíba', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      17: {'name': 'Paraná', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      18: {'name': 'Pernambuco', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      19: {'name': 'Piauí', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      20: {'name': 'Rio de Janeiro', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      21: {'name': 'Rio Grande do Norte', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      221: {'name': 'Rio Grande do Sul', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      231: {'name': 'Rondônia', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      241: {'name': 'Roraima', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      251: {'name': 'Santa Catarina', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      281: {'name': 'Sergipe', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      271: {'name': 'Tocantins', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
        }

    if region is 'Sao Paulo':
        location_map.pop('1')
        region = 'São Paulo'
    elif region is 'Rio De Janeiro':
        location_map.pop('2')
    if region is 'Acre':
        location_map.pop('3')

    location_map_br_11_cap = {
  1: {'name': 'Toda Região 11 (ddd)', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
  2: {'name': 'Todo Estado de São Paulo', 'lat': -22.90, 'long': -43.21, 'radius': 294200},
  1: {'name': 'Toda região Sudeste do Brasil', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
    }

    location_map_br_11 = {
      232: {'name': '- Zona Centro', 'lat': -22.90, 'long': -43.21, 'radius': 294200},
      133: {'name': '- Zona Norte', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      12: {'name': '- Zona Sul', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      111: {'name': '- Zona Leste', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      121: {'name': '- Zona Oeste', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      12223: {'name': 'Atibaia', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      14: {'name': 'Barueri', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      16: {'name': 'Bragança Paulista', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      17: {'name': 'Carapicuíba', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      18: {'name': 'Cotia', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      19: {'name': 'Diadema', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      10: {'name': 'Embu', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      21: {'name': 'Ferraz de Vasconcelos', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      221: {'name': 'Francisco Morato', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      231: {'name': 'Franco da Rocha', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      241: {'name': 'Guarulhos', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      251: {'name': 'Itapecerica da Serra', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      261: {'name': 'Itaquaquecetuba', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      271: {'name': 'Itu', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      281: {'name': 'Jundiaí', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      291: {'name': 'Mauá', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      311: {'name': 'Poá', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      321: {'name': 'Ribeirão Pires', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      331: {'name': 'Santana de Parnaíba', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      341: {'name': 'Santo André', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      351: {'name': 'São Bernardo do Campo', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      361: {'name': 'São Caetano do Sul', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      371: {'name': 'Suzano', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      381: {'name': 'Taboão da Serra', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
      391: {'name': 'Várzea Paulista', 'lat': -23.55, 'long': -46.64, 'radius': 294200},
    }

        #order_by = self.request.get('order')
        """
        query = Owner.all()
        query.filter("owner =", user)
        if not order_by:
            query.order("owner_tag")
        elif order_by == 'date':
            query.order("-date")
        ...
        <a href="your_page_url?order=date">order tags by most recent</a>
        """

    #query = Ad.all().search(self.request.get('q')).filter("url IN", ['www.montao.com.br','montao']).filter("modified >", timeline).filter("published =", True).order("-modified")


        ads = Ad.proximity_fetch(Ad.all().search(self.request.get('q')).filter("modified >", timeline).filter("published =", True).order("-modified") ,db.GeoPt(lat, lon),max_results=PAGESIZE+1, max_distance=m)
        ads = sorted(ads, key=lambda x: x.modified, reverse=True)
        if ads and len(ads) == PAGESIZE+1:
            next = ads[-1].modified
            ads = ads[:PAGESIZE]

        paginator = Paginator(ads,PAGESIZE)
        ads = paginator.page(page)

        location_map = sorted(location_map.iteritems(), key=lambda x: x[1]["name"])
        location_map_br_11_cap = sorted(location_map_br_11_cap.iteritems(), key=lambda x: x[1]["name"])
        location_map_br_11 = sorted(location_map_br_11.iteritems(), key=lambda x: x[1]["name"])

        all_count = Ad.all().filter("modified >", timeline).filter("published =", True).count(100000000)
        private_count = Ad.all().filter("modified >", timeline).filter("published =", True).filter("company_ad =", False).count(100000000)
        company_count = Ad.all().filter("modified >", timeline).filter("published =", True).filter("company_ad =", True).count(100000000)

        to = PAGESIZE * page
        fromand = to - PAGESIZE + 1
        if to > all_count:
            to = all_count

        self.render(u'for_sale_br', country='Brazil', region=region, category=category, city=city, location_map_br_11_cap = location_map_br_11_cap, location_map_br_11 = location_map_br_11, to=to, fromand=fromand, all_count=all_count, private_count=private_count, company_count=company_count,location_map=location_map, paginator=paginator,twittername=twittername,request=self.request,lat=self.request.get('lat'),lon=self.request.get('lon'),q=q,logo=logo,ads=ads, user_url=users.create_logout_url(self.request.uri) if users.get_current_user() else None, admin=users.is_current_user_admin(),user=users.get_current_user() if users.get_current_user() else None, )

application = webapp.WSGIApplication([('/([^/]+)/?([^/]*)/?([^/]*)',Handler),],debug=True)

def main():
    logging.getLogger().setLevel(logging.DEBUG)
    run_wsgi_app(application)

if __name__ == "__main__":
    main()
\$\endgroup\$
2
\$\begingroup\$

Your imports could use some cleanup, you import that same thing a few different times. I also suspect you aren't using some of the stuff you are importing, so yeah I suggest some effort to clean that up.

Your indentation is messed up on here, making it harder to read. Probably a copy/paste error somewhere along the way.

Your big dictionaries of data should be global variables or loaded from a seperate file.

if region is city:, I don't think this means what you think it means. It compares the object identity not the contents of the string. It will always come out false in this peice of code. I think you meant ==

region = region.replace("_"," ").title()

You do this basic operation several times, put it in a reusable and well-named function.

page = int(self.request.GET.get('page', '1'))

What if the web request has something that is not a number in there? You've gotta be careful with user input.

m=int(self.request.get('r')) if self.request.get('r') else 804670

m is a very mysterious variable name, it doesn't give much hints as to what you are doing. Also, if self.request is a dictionary, get takes a second parameter which is a default.

if region is 'Sao Paulo':
    location_map.pop('1')
    region = 'São Paulo'
elif region is 'Rio De Janeiro':
    location_map.pop('2')
if region is 'Acre':
    location_map.pop('3')

is doesn't do what you want here, if this has escaped your notice until now, you seriously need to improve your testing practices.

#order_by = self.request.get('order')
    """
    query = Owner.all()
    query.filter("owner =", user)
    if not order_by:
        query.order("owner_tag")
    elif order_by == 'date':
        query.order("-date")
    ...
    <a href="your_page_url?order=date">order tags by most recent</a>
    """

#query = Ad.all().search(self.request.get('q')).filter("url IN", ['www.montao.com.br','montao']).filter("modified >", timeline).filter("published =", True).order("-modified")

Don't leave commented code in your code. It clutters the code.

    ads = Ad.proximity_fetch(Ad.all().search(self.request.get('q')).filter("modified >", timeline).filter("published =", True).order("-modified") ,db.GeoPt(lat, lon),max_results=PAGESIZE+1, max_distance=m)

Line is seriously too long. Look at breaking it up over several lines

    ads = sorted(ads, key=lambda x: x.modified, reverse=True)

if ads has an order specified, why are you sorting?

    if ads and len(ads) == PAGESIZE+1:
        next = ads[-1].modified
        ads = ads[:PAGESIZE]

next is a builtin python function. Consider using another name

\$\endgroup\$
  • \$\begingroup\$ Thank for the elaborate answer. I created a seperate question for the imports on SO and will take the measure to handle everything you mention to improve my code. \$\endgroup\$ – Niklas Sep 26 '11 at 17:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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