Created
March 30, 2010 18:23
-
-
Save richtaur/349393 to your computer and use it in GitHub Desktop.
critique my python!
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#!/usr/bin/env python | |
import cgi | |
import simplejson as json | |
import os | |
from google.appengine.ext import db | |
from google.appengine.ext import webapp | |
from google.appengine.ext.webapp import template | |
from google.appengine.ext.webapp import util | |
NAME_MIN = 2 | |
NAME_MAX = 50 | |
class Score(db.Model): | |
date = db.DateTimeProperty(auto_now_add = True) | |
name = db.StringProperty(multiline = False) | |
score = db.IntegerProperty() | |
class MainHandler(webapp.RequestHandler): | |
def get(self): | |
scores = db.GqlQuery('SELECT * FROM Score ORDER BY score DESC LIMIT 10') | |
template_values = { | |
'scores' : scores | |
} | |
path = os.path.join(os.path.dirname(__file__), '../tpl/home.tpl') | |
self.response.out.write(template.render(path, template_values)) | |
class APIHandler(webapp.RequestHandler): | |
def get(self): | |
self.response.headers['Content-Type'] = 'application/json' | |
self.response.out.write(json.dumps({ | |
'message' : 'Bombada high score API.' | |
})); | |
def post(self): | |
self.response.headers['Content-Type'] = 'application/json' | |
name = self.request.get('name') | |
score = self.request.get('score') | |
if (name) and (score): | |
if (not self.valid_name(name)): | |
self.response.out.write(json.dumps({ | |
'error' : True, | |
'message' : 'name must be %i-%i characters' % (NAME_MIN, NAME_MAX) | |
})); | |
return | |
if (not self.valid_score(score)): | |
self.response.out.write(json.dumps({ | |
'error' : True, | |
'message' : 'Invalid score.' | |
})); | |
return | |
else: | |
self.response.out.write(json.dumps({ | |
'error' : True, | |
'message' : 'name and score are required fields.' | |
})); | |
return | |
score_row = Score() | |
score_row.name = name | |
score_row.score = int(score) | |
score_row.put() | |
self.response.out.write(json.dumps({ | |
'message' : 'Recorded: %s (%s)' % (name, score) | |
})); | |
# Checks if a name is valid. | |
# @param {str} name The name to check. | |
# @return {bool} True if the name is valid, otherwise False. | |
def valid_name(self, name): | |
length = len(name); | |
return (length >= NAME_MIN) and (length <= NAME_MAX) | |
# Checks if a score is valid. | |
# @param {int} score The score to check. | |
# @return {bool} True if the score is valid, otherwise False. | |
def valid_score(self, score): | |
if (score and score.isdigit()): | |
return (int(score) > 0) | |
else: | |
return False | |
def main(): | |
application = webapp.WSGIApplication( | |
[ | |
('/', MainHandler), | |
('/api', APIHandler) | |
], | |
debug = True | |
) | |
util.run_wsgi_app(application) | |
if (__name__ == '__main__'): | |
main() |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I forked this here to show the changes I'd make to it. There's nothing inherently wrong about your code, but it's not how Python is typically written. It's mostly style choices, so feel free to take from it what you like. Most of it is PEP-8 and the rest is personal preference.
One change I didn't make is the conversion from tabs to spaces. You'll find most Python programmers are adamant about using spaces instead of tabs, but I know you're on the other side, so I didn't change it. Besides, if you diff the two, it'll be easier to see the other differences if there's not annoying whitespace changes on every single line.
Also, I'm not sure what format the method comments were supposed to be, so I took a shot in the dark when converting them to use exceptions. Ignore that if it's wrong. Though I should note that things like that are typically done in docstrings instead of comments, so automated tools can see them too. If you're already using something that knows how to deal with comments instead, though, that works.