Monthly rollover system

I have learned a little Python for a few projects at work. This is my first finished application and I was hoping someone would take a look at it. I would love some tips on the mistakes I've made.

```
################################################################################
#Monthly Rollover System #
#Written by Ryan Hanson 2012 #
# #
################################################################################

#Python Imports
import sys

#PyQt4 Imports
from PyQt4 import QtCore, QtGui, QtSql #@UnusedImport

#UI Imports
from main import Ui_MainWindow
from popup import Ui_Dialog

#Variables
dbhost = "192.168.1.39"
dbname = "material"
dbuser = "fab"
dbpswd = "doylefab"

#This Runs First
class launch(QtGui.QMainWindow):
def __init__(self, parent=None):
QtGui.QMainWindow.__init__(self, parent)

#Prepare the first window
self.main = Ui_MainWindow()
self.main.setupUi(self)

#Set the icon and select the first tab
self.setWindowIcon(QtGui.QIcon("icons/main.png"))
self.setWindowTitle("Material Tracking")
self.main.tabs.setCurrentIndex(0)

#Make db accessible to the whole app - I am not sure this is the best way to do this?
global db
db = QtSql.QSqlDatabase.addDatabase("QMYSQL");

#Host name and Database
db.setHostName(dbhost)
db.setDatabaseName(dbname)
db.setUserName(dbuser)
db.setPassword(dbpswd)
ok = db.open()

#ok is true if the database can be connected to
if ok:
self.materialQuery = QtSql.QSqlQuery(db)
materialdata = "select id, material, d1, d2, d3 from material_tbl"
if self.materialQuery.exec_(materialdata):
self.materialchange(1)

#Set the range for the slid

Some really obvious stuff popped up in a casual pylint check over your code. As well as some further, more advanced ideas for changing your code below.

db = QtSql.QSqlDatabase.addDatabase("QMYSQL");

The semi-colon is not necessary in python.

while self.materialQuery.value(0).toString() <> kid:
if self.main.lineSearch <> "":
if self.main.lineSearch <> "":

The <> operator is frowned upon, and is going to be removed. Your code will not be forwards-compatible.

r=0
R=self.main.modelMaterial.rowCount()
c=0
C=self.main.modelMaterial.columnCount()
s=0
s==C:
c<C:

PEP8 standard specifies that you have to have spaces before and after assignment, equality and comparison operators, among others. You're not getting any benefit if you scrunch stuff up like that. This isn't C.

#Is there a better way to set columns?
self.main.tableMaterial.setColumnWidth(0, 50)
self.main.tableMaterial.setColumnWidth(1, 200)
self.main.tableMaterial.setColumnWidth(2, 75)
self.main.tableMaterial.setColumnWidth(3, 100)
self.main.tableMaterial.setColumnWidth(4, 100)
self.main.tableMaterial.setColumnWidth(5, 100)

Yes, it's called a loop. Such as:

COL_WIDTHS_CONST = {
  0: 50,
  1: 200,
  2: 75,
  3: 100,
  4: 100,
  5: 100,
}
def set_widths(self, colDict):
    for column in colDict:
        self.main.tableMaterial.setColumnWidth(column , colDict[column])

# Somewhere in your init code:
self.set_widths(self, COL_WIDTHS_CONST):

Move stuff out of your init method. Even if all the magic happens in the init method, you can't put all the low-level code there. Make separate functions. e.g.

• SetupMainWindow

• SetupDatabase

• GetMaterialData (From the database)

• ConnectEvents

• ConfigureGUIElements (You put your column/row width setting code here)

The fact that you had so many comments in your giant 40 line init method should be telling of the fact that it was doing too much in one spot, and you had to use comments to divide it into separate groupings.

Another thing I noticed. You seem to be mixing your data retrieval, presentation and control logic all in the same pieces of code. You should move code out into separate functions. If you have a method that is more than a few lines, then odds are it does too much, or it repeats.

Move the SQL code out of your code, and into constants. It makes making changes to your SQL a little more manageable.

One last thing. It's something I see often with sloppy coders that don't use a proper IDE for python development. Trailing whitespace.

StackExchange Code Review Q#12619, answer score: 3