Jimbo wrote:
I have made a Python App(really script) that will check a stocks
current values from a website & save that data to a SQLite 3 database.
I am looking for any suggestions & criticisms on what I should do
better or anything at all but mainly in these areas:
[QUOTE]
- Correct Python Layout of code
- Correct error checking: Am I catching all my errors or are my
exceptions not specific enough? Should I be using more try excepts
inside my functions?
- Are there any areas where huge errors, bugs etc could occur that I
have not compensated for?
- I am also looking for suggestions on how to write a function better,
so if you think that function is really bad & should be rewritten
completely or something I would really like to hear it.
- Anything else that you see
- Is python meant to be used in the way I have used it? To make a
'semi detailed' app?[/QUOTE]
Any advice criticism would be really helpful.
App:
[CODE]"""
*Stock Data Builder*
Algorithm:
- Search website for stock
- Get website HTML source code
- Search code for target stock data(price,dividends,etc..)
- Add data to text file
"""
import sys
import os
import sqlite3
import datetime
import time
import urllib2
### Global Variables ###
menu = "***Stock Program*** \n\n1. Add a Stock to track \n2. Get
Todays Tracking Data \n3. Exit \nEnter decision: "
target = '<th scope="row" class="row"><a href="/asx/research/
companyInfo.do?by=asxCode&asxCode=%s">%s</a>'
ASXurl = 'http://www.asx.com.au/asx/markets/priceLookup.do?
by=asxCodes&asxCodes='
class stock:
code = ""
purchasePrice = 0
purchaseQuantity = 0
price = [] # list of recent prices
recentBid = [] # list of recent bids for stock
recentOffer = [] # list of recent offers for stock
stockVol = [] # list of stock quantity available on
market
This will be variables belonging to the class itself, not instances of
it.
def __init__(self):
""" Default Constructor """
self.code = ""
self.purchasePrice = 0
self.purchaseQuantity = 0
def constructor(self, stockCode, purPrice, purQuant):
""" Constructor """
self.code = stockCode
self.purchasePrice = purPrice
self.purchaseQuantity = purQuant
def setData(self, stockCode, purPrice, purQuant, priceList,
reBidList, reOffList, popList):
""" Defines & implements the objects' public variables """
self.code = stockCode
self.purchasePrice = purPrice
self.purchaseQuantity = purQuant
self.price = priceList
self.recentBid = reBidList
self.recentOffer = reOffList
self.stockVol = popList
self.printStats()
def updateData(self, priceEle, bidEle, offerEle, populEle):
""" Adds data to stock object's lists """
self.price.append(priceEle)
self.recentBid.append(bidEle)
self.recentOffer.append(offerEle)
self.stockVol.append(populEle)
def printStats(self):
""" Output Stock attributes """
print("Stock Code: "+self.code)
In Python 2 'print' is a statement, so it doesn't need its arguments to
be enclosed in (). You could also use Python's string formatting:
print "Stock Code: %s" % self.code
print("Stock Purchase Price: "+str(self.purchasePrice))
print("Stock Quantity Owned: "+str(self.purchaseQuantity))
print("***Initial Investment Value:
"+str(self.purchasePrice*self.purchaseQuantity))
if not(len(self.price) <= 0):
'not' has a lower priority than '<=', and this can be simplified anyway:
if len(self.price) > 0:
or even:
if self.price:
because empty containers (eg lists) are treated as False, non-empty ones
as True, by 'if' and 'while' statements.
print("Stock Current Price: "+str(self.price[-1]))
print("Recent Bid: "+str(self.recentBid[-1]))
print("Recent Offer: "+str(self.recentOffer[-1]))
print("Total Stock Volume in market:
"+str(self.stockVol[-1]))
print("***Present Investment Value:
"+str(self.price[-1]*self.purchaseQuantity))
print("\n")
### Functions ###
def connectDatabase(dbLocation, dbName, tableName):
""" Establish & Return connection to SQLite Database """
try:
if not (os.path.exists(dbLocation)):
os.mkdir(dbLocation) # create folder/dir
os.chdir(dbLocation) # change directory focus to
dbLocation
It's normally easier to use absolute paths instead of changing the
current directory.
conn = sqlite3.connect(dbLocation+dbName)
It's better to join paths using os.path.join() because that will insert
any directory separators for you.
cur = conn.cursor()
try:
createTableQ = "CREATE TABLE IF NOT EXISTS "+tableName
+" (code varchar PRIMARY KEY, purchase_price float, purchase_quantity
float, purchase_date varchar);"
cur.execute(createTableQ)
conn.commit()
except:
pass
DON'T USE BARE EXCEPTS, especially if you're just going to ignore the
exception. Exceptions can be raised for a variety of reasons, including
one you didn't expect due to bugs, so catch only what you expect.
return conn
except IOError or OSError:
The result of:
IOError or OSError
is:
IOError
What you want is:
except (IOError, OSError):
Note: the parentheses are necessary here, otherwise it it would mean
"catch an IOError exception and then bind it to the name 'OSError'".
print "Connection to database failed"
return False
Some times you return the connection, and other times False. A more
usual value than False in such cases is None (unless you're returning
True or False).
def retrieveStockDatabase(conn, tableName):
""" Read SQLite3 database & extract stock data into StockList """
stockList = []
stockQuery = "select recent_price, recent_offer, recent_bid,
stock_volume from ? ;"
cur = conn.cursor()
cur.execute("select code, purchase_price, purchase_quantity from
"+tableName+";")
for row in cur.fetchall():
newStock = stock()
newStock.code = row[0]
newStock.purchasePrice = row[1]
newStock.purchaseQuantity = row[2]
cur.execute(stockQuery,[newStock.code])
for rw in cur.fetchall():
newStock.price.append(rw[0])
newStock.recentOffer.append(rw[1])
newStock.recentBid.append(rw[2])
newStock.stockVol.append(rw[3])
stockList.append(newStock)
return stockList
def getDate():
""" Return todays date in format DD:MM:YYYY """
time = datetime.datetime.now()
date = time.strftime("%d:%m:%Y") # string format time (%y)
return date
def newStockDatabase(conn, stockTable, stock):
""" Add a new stock to SQLite database if not already there
We save the stocks code, purchase price, quantity purchased
& date of purchase. """
cur = conn.cursor()
try:
createTableQ = "create table "+stock.code+" (date varchar
PRIMARY KEY, recent_price float, recent_offer float, recent_bid float,
stock_volume double);"
stockQuery = "insert into "+stockTable+"
values(?, ?, ?, ?);"
cur.execute(createTableQ)
cur.execute(stockQuery,
[stock.code,stock.purchasePrice,stock.purchaseQuant,getDate()])
conn.commit()
except IOError or OSError:
print "Table may already exist or bad SQLite connection."
return False
def webFormat(URL):
if (URL.startswith("http://";)==False):
URL = "http://"+URL
Better as:
if not URL.startswith("http://";):
The conditions of 'if' and 'while' statements don't need to enclosed in
parentheses like in C, Java, etc.
return URL
def getSource(URL):
""" Retrieve HTML source code from website URL &
save in sourceBuffer """
try:
URL = webFormat(URL) # make sure URL contains essential
"http://";
sourceBuffer = urllib2.urlopen(URL)
print '\nResponse code = ',sourceBuffer.code
print 'Response headers = ',sourceBuffer.info()
print 'Actual URL = ',sourceBuffer.geturl()
sourceCode = sourceBuffer.read()
sourceBuffer.close()
return sourceCode
except IOError: # URLError
print "Function Failed: Reasons could be invalid URL name \nOR
\nHTML protocol message transfer failure."
return False # function failed
def getTargetText(targetStrtData, targetEndData, dataBuffer):
""" Grabs target text that lies inside 'dataBuffer' string
between targetStrtData & targetEndData """
try:
result = dataBuffer.split(targetStrtData)
result.pop(0)
result = result[0].split(targetEndData)
result.pop(1)
print result
return result
except IOError:
print "Function Failed: Reasons could be targetStrtData and/or
targetEndData is not present in dataBuffer."
You haven't given a return statement for when there's an error, so it'll
return None. It's probably clearer if you write a function in Python
either as a _function_ which _always_ returns a value or as a
_procedure_ which _never_ returns a value.
In Python the preference is for a function to raise an exception to
indicate an error instead of returning a flag.
def getStockData(htmlText, selectedStock):
""" Extract stock data(stock code,price,etc) from htmlText """
try:
# Here I extract my number data from HTML text
tempList = []
for string in htmlText:
for i in string.split('>'):
for e in i.split():
if ('.' in e and e[0].isdigit()):
tempList.append(float(e))
elif (',' in e and e[0].isdigit() ):
# remove ',' chars
e = e.replace(',','')
tempList.append(float(e))
selectedStock.updateData(tempList[0],tempList[2],tempList[3],tempList[7])
except IOError: # is this the correct error I should be trying to
catch here??
print "Function Failed: Reasons could be: sites HTML data has
changed. Consult author of program."
return False
def createStockTracker(stockCode,stockPrice,stockQuant, stockList):
""" Add a new stock to the database to track """
newStock = stock()
newStock.constructor(stockCode,stockPrice,stockQuant)
stockList.append(newStock)
return stockList
def writeStockToDatabase(conn, stock):
""" Write ONLY this Stock's attributes to SQLite Database """
cur = conn.cursor()
date = getDate()
tableName = stock.code
stockquery = "insert into "+tableName+" values(?, ?, ?, ?, ?);"
cur.execute(query,[date,stock.price[-1], stock.recentOffer[-1],
stock.recentBid[-1], stock.stockVol[-1]])
conn.commit()
def writeAllToDatabase(conn, stockList):
""" Enter recent Stock attributes into SQLite Database """
cur = conn.cursor()
date = getDate()
for stock in stockList:
tableName = stock.code
stockquery = "insert into "+tableName+"
values(?, ?, ?, ?, ?);"
cur.execute(query,[date,stock.price[-1],
stock.recentOffer[-1], stock.recentBid[-1], stock.stockVol[-1]])
conn.commit()
### Input Functions ###
def inputNewStock():
""" """
print "*Please note only an Australian Securities Exchange(ASX)
listed stock can be tracked in Version 1.0."
badInput = True
while (badInput == True):
Better as:
while badInput:
try:
code = raw_input("Please enter the ASX code for the
stock you wish to track: ")
If you assign to a name in a function then that name will be treated as
a local name (variable) unless you say it's global in the function,
therefore 'code', etc, won't been seen outside this function.
price = input("Please enter the individual stock value
for "+code+": ")
quantity = input("Please enter the number/quantity of
stocks purchased: ")
if (len(code)>3):
badInput = True
else : badInput = False
return True
This will check the length, setting badInput accordingly, and then
return True.
except IOError:
I don't see anything in the 'try' block that will raise IOError.
if (raw_input("Incorrect input. Note: ASX code cannot be
more than 3 chars. Press 'x' to exit or anything else to try
again")=='x'):
return False
badInput = True # this I am not sure if necessary to loop
correctly
You're not explicitly returning anything from the function when the loop
exits, so it would return None.
### Main program loop ###
def main():
programEnd = False;
dbLocation = "C:\Users\Sam\Desktop\StockApp/"
If your string contains literal backslashes then use raw strings
instead. Raw strings can end in a backslash, but that shouldn't matter
in the script if you're joining paths together using os.path.join().
dbName = "stockData.db"
stockTable = "stocks"
conn = connectDatabase(dbLocation,dbName,stockTable)
stockList = retrieveStockDatabase(conn,stockTable)
for s in stockList:
s.printStats()
while (programEnd == False):
Better as:
while not programEnd:
decision = input(menu) # Print Menu
input() is often considered a bad function to use because it's
equivalent to eval(raw_input()), so the user could enter any expression.
It's better to use int(raw_input()) in this case and then catch the
ValueError if what was entered isn't an int.
if (decision==1):
if not(inputNewStock()==False):
A double-negative, equivalent to:
if inputNewStock() != False:
It would be more Pythonic if you had inputNewStock() return the code,
etc, or raise an exception if there was an error. You could then catch
that exception in a 'try' block.
stockList =
createStockTracker(acode,price,quantity,stockList)
newStockDatabase(conn,stockTable,stockList[-1])
print("\n** New Stock **")
stockList[-1].printStats()
print "The stock "+code+" was successfully added to
our database. \nNow every time the program runs it will automatically
track this stock & obtain its stock attributes\n\n"
# TO DO:
# get new stock recent Data from internet etc.
# add stock data to data base;
elif (decision==2):
if (len(stockList)>0):
Better as:
if stockList:
for Stock in stockList:
URL = ASXurl+Stock.code
sourceCode = getSource(URL)
targetData = getTargetText(target %
(Stock.code,Stock.code),"</tr>",sourceCode)
getStockData(targetData,Stock)
Stock.printStats()
#writeStockToDatabase(conn,Stock)
else:
print "You must first identify a stock to follow.
\nPlease select option 1."
elif (decision==3):
print "Thank you for using Stock Program. Goodbye.\n"
programEnd = True; # Exit program
conn.close()
return 0 # End program
'main' is called by the main program and its return value is never used,
so just omit this return statement.
main()[/CODE]
The recommended standard in Python is for names of classes to be
CamelCase, names of constants to be UPPERCASE_WITH_UNDERSCORES, and
names of everything else to be lowercase_with_underscores.
--
http://mail.python.org/mailman/listinfo/python-list
