HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonflaskMinor

Contest assist web app

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Opening files using with

It'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 object

In 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, not PascalCase, or camelCase.



  • Variable names should be in snake_case as well.



  • Constants should be in UPPER_SNAKE_CASE



  • The XMLParser class should be named XmlParser.

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.