2
\$\begingroup\$

I'm currently working with TornadoWeb but appear to have picked up a lot of bad habbits. I'd appreciate some feedback on the code below:

import tornado.ioloop
import tornado.web
import tornado.template

#modules
import auth
import base
import tag

#pymongo
import pymongo
connection = pymongo.Connection("127.0.0.1", 27017)
db = connection.prod
loader = tornado.template.Loader("templates")
class CustomerJS(tornado.web.RequestHandler):
    def get(self):
        ckey = self.get_secure_cookie('ckey')
        self.set_header("Content-Type", "Application/x-javascript")
        self.write(loader.load('js/customer.js').generate(ckey=ckey))


class Account(base.Base):
    def get(self):
        self.render('account.html', None)

class Sites(base.Base):
    def get(self):
        self.render('sites.html', None)

class Ads(base.Base):
    def get(self):
        self.render('ads.html', None)

application = tornado.web.Application([
        ## MAIN TABS
        # home
        (r"/version/3/home", Account),
        # ads
        (r"/version/3/ads", Ads),
        (r"/version/3/tags", tag.Tags),
        (r"/version/3/tagPost", tag.tagPost),
        (r"/version/3/tagRemove", tag.tagRemove),
        (r"/version/3/tagReportCookies", tag.tagReportCookies),
            (r"/version/3/s/(.*)", tornado.web.StaticFileHandler, {"path": "static"}),
            # JAVASCRIPT
            (r"/version/3/customer.js", CustomerJS),
            ], cookie_secret="COOKIESECRET=")

if __name__ == "__main__":
    application.listen(9099)
    tornado.ioloop.IOLoop.instance().start()
\$\endgroup\$
1
\$\begingroup\$
import tornado.ioloop
import tornado.web
import tornado.template

#modules

This comment doesn't say much, its fairly clear that anything starting with import is a module. It would be better to say something about why these three modules are grouped - eg, 'http/html modules'. But for a low number of imports, you mightn't bother with the comment at all.

import auth
import base
import tag

#pymongo

Same again; this comment doesn't say anything that the code doesn't. This one you can just drop.

import pymongo

Put a blank line below your last import. Also, where are auth, tag and base from? They sound like your own support modules rather than part of a third-party package, in which case the imports should go after pymongo, rather than before, per PEP 8.

connection = pymongo.Connection("127.0.0.1", 27017)
db = connection.prod
loader = tornado.template.Loader("templates")
class CustomerJS(tornado.web.RequestHandler):
    def get(self):
        ckey = self.get_secure_cookie('ckey')

Stick to either single or double quotes throughout your code, rather than swapping between them.

        self.set_header("Content-Type", "Application/x-javascript")
        self.write(loader.load('js/customer.js').generate(ckey=ckey))


class Account(base.Base):

If base is your own module, you should pick a better name for it.

    def get(self):
        self.render('account.html', None)

class Sites(base.Base):
    def get(self):
        self.render('sites.html', None)

class Ads(base.Base):
    def get(self):
        self.render('ads.html', None)

These three classes differ only by one hard-coded string argument. I'm assuming these are going to be expanded a bit later, but even so - consider consolidating them.

application = tornado.web.Application([
        ## MAIN TABS
        # home

Drop this comment, its clear that its the home page from the next line.

        (r"/version/3/home", Account),

Is there a reason why all of these strings are raw strings?

        # ads

Does this comment really apply to the entire next block of code (including the bits dealing with Tags and tagPost, etc)? If it doesn't, the comment is not only redundant but actually misleading.

        (r"/version/3/ads", Ads),
        (r"/version/3/tags", tag.Tags),
        (r"/version/3/tagPost", tag.tagPost),
        (r"/version/3/tagRemove", tag.tagRemove),
        (r"/version/3/tagReportCookies", tag.tagReportCookies),
            (r"/version/3/s/(.*)", tornado.web.StaticFileHandler, {"path": "static"}),

Why do you suddenly indent further from here?

        # JAVASCRIPT

Put a blank line before this.

        (r"/version/3/customer.js", CustomerJS),
        ], cookie_secret="COOKIESECRET=")

You've passed a long list as the first argument to Application, and then a single string value as a second - but the way you've set it out, it could be mistaken as sending many arguments to Application. Better to do it this way:

Application(
     [ # Main tabs
       (r"/version/3/home", Account),
       (r"/version/3/ads", Ads),
            ...
      # Javascript
       (r"/version/3/customer.js", CustomerJS),
      ],
      cookie_secret="COOKIESECRET="
)

Or something similar, to make it obvious what your code is actually doing.

if __name__ == "__main__":
    application.listen(9099)
    tornado.ioloop.IOLoop.instance().start()
| improve this answer | |
\$\endgroup\$

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.