patternpythonflaskMinor
Contest assist web app
Viewed 0 times
assistcontestwebapp
Problem
This is a small web application that I have created using the Flask framework.
I am looking for best practices feedback, as well as design pointers. I am not confident that my design choices have been very good, or rather that they were the best they could have been, but I would like some feedback regardless on where I can improve and expand as I'm new to Python.
To begin, I'd like to review where I process my XML configuration file. The file is validated, and then a database is populated with the information. I feel like it could be improved, however, not included is validation and safety-checking of the user-supplied (really admin supplied) HTML from the XML doc.
```
from lxml import etree
import time, datetime
from time import mktime
from contestassist.database import db_session
from contestassist.contest import Contest
from contestassist.question import Question
from contestassist.config import Config
import sqlalchemy
import sys
class XMLParser:
def __init__(self, xml_file):
self.xml_file = xml_file
self.time_format = "%Y-%m-%d %H:%M:%S"
def ProcessTemplate(self):
#parser = etree.XMLParser(dtd_validation=True)
parser = etree.XMLParser()
xmldoc = etree.parse(self.xml_file, parser)
contests = xmldoc.getroot()
#validate against DTD
dtd = open(Config.DTDPATH, 'r')
dtdObj = etree.DTD(dtd)
if dtdObj.validate(contests) is False:
print "Supplied XML file is not correctly formatted"
print(dtdObj.error_log.filter_from_errors()[0])
sys.exit()
for contest in contests.getchildren():
contest_id = self.parseContestInfo(contest)
self.parseContestQuestions(contest, contest_id)
def parseContestInfo(self, contestNodes):
title = contestNodes.xpath('title')
self.title_data = title[0].text
description = contestNodes.xpath('description')
self.description_data = description[0].text
I am looking for best practices feedback, as well as design pointers. I am not confident that my design choices have been very good, or rather that they were the best they could have been, but I would like some feedback regardless on where I can improve and expand as I'm new to Python.
To begin, I'd like to review where I process my XML configuration file. The file is validated, and then a database is populated with the information. I feel like it could be improved, however, not included is validation and safety-checking of the user-supplied (really admin supplied) HTML from the XML doc.
```
from lxml import etree
import time, datetime
from time import mktime
from contestassist.database import db_session
from contestassist.contest import Contest
from contestassist.question import Question
from contestassist.config import Config
import sqlalchemy
import sys
class XMLParser:
def __init__(self, xml_file):
self.xml_file = xml_file
self.time_format = "%Y-%m-%d %H:%M:%S"
def ProcessTemplate(self):
#parser = etree.XMLParser(dtd_validation=True)
parser = etree.XMLParser()
xmldoc = etree.parse(self.xml_file, parser)
contests = xmldoc.getroot()
#validate against DTD
dtd = open(Config.DTDPATH, 'r')
dtdObj = etree.DTD(dtd)
if dtdObj.validate(contests) is False:
print "Supplied XML file is not correctly formatted"
print(dtdObj.error_log.filter_from_errors()[0])
sys.exit()
for contest in contests.getchildren():
contest_id = self.parseContestInfo(contest)
self.parseContestQuestions(contest, contest_id)
def parseContestInfo(self, contestNodes):
title = contestNodes.xpath('title')
self.title_data = title[0].text
description = contestNodes.xpath('description')
self.description_data = description[0].text
Solution
Opening files using
It's not a great idea to open a file just using something like this:
If your program unexpectedly exits, or crashes, resources that are taken up by the opened file aren't freed. This can be bad in some situations. The proper way to open a file would to be using a context manager, or
By using a context manager, resources will be properly freed even if your program crashes or unexpectedly exits.
Explicit inheritance from
In addition, classes in pre-Python 3.x should explicitly inherit from
When your class doesn't you can get weird behavior like this:
Versus normal behavior like this:
Nitpicks/Style
Python has an official style guide, PEP8. I'd highly recommend that you read it, as it has some very valuable information. You do have a few style violations though, so here's a small list of them:
withIt's not a great idea to open a file just using something like this:
dtd = open(Config.DTDPATH, 'r')If your program unexpectedly exits, or crashes, resources that are taken up by the opened file aren't freed. This can be bad in some situations. The proper way to open a file would to be using a context manager, or
with, like this:with open(Config.DTDPATH, "r") as dtd:
...By using a context manager, resources will be properly freed even if your program crashes or unexpectedly exits.
Explicit inheritance from
objectIn addition, classes in pre-Python 3.x should explicitly inherit from
object, like this:class XmlParser(object):
...When your class doesn't you can get weird behavior like this:
>>> class Spam: pass
...
>>> type(Spam())
Versus normal behavior like this:
>>> class Spam(object): pass
...
>>> type(Spam())
Nitpicks/Style
Python has an official style guide, PEP8. I'd highly recommend that you read it, as it has some very valuable information. You do have a few style violations though, so here's a small list of them:
- Function names should be in
snake_case, notPascalCase, orcamelCase.
- Variable names should be in
snake_caseas well.
- Constants should be in
UPPER_SNAKE_CASE
- The
XMLParserclass should be namedXmlParser.
Code Snippets
dtd = open(Config.DTDPATH, 'r')with open(Config.DTDPATH, "r") as dtd:
...class XmlParser(object):
...>>> class Spam: pass
...
>>> type(Spam())
<type 'instance'>>>> class Spam(object): pass
...
>>> type(Spam())
<class '__main__.Spam'>Context
StackExchange Code Review Q#7796, answer score: 3
Revisions (0)
No revisions yet.