2
\$\begingroup\$

In short, I need help simplifying and optimizing this code because, now that I have switched to a tree widget from 2 list widgets, loading the tables has increased by roughly 3-4 seconds (it used to be almost instant). I would also like to shorten this function if possible.

--- Background Info ---
self.tableData -- The table that the information is being displayed to
getItem() -- a function that when a table of a database is selected, returns the database name (var[0]) and the table name (var[1])
columnNames -- Returns the column names for the table
rows -- returns the total number of rows in the table
g -- index value for the row
h -- index value for the column the data will go in
-- forgot what i, j, k do because I was working on other parts of the program, but they are needed (been some time now)

Note: This code belongs to a class which I have not included

def LoadTable():
 self.tableData.clear()
 if getItem() != None:
 var = getItem()
 try:
 ccnnxx = mysql.connector.connect(user=self.uname.text(), password=self.upass.text(), host=self.uhost.text(), port=self.uport.text(), database = var[0])
 cursor = ccnnxx.cursor()
 self.tableData.setRowCount(0)
 self.tableData.setColumnCount(0)
 header = self.tableData.horizontalHeader() 
 item = getItem()
 cursor.execute("SELECT * FROM " + var[1])
 rows = cursor.fetchall()
 self.tableData.setRowCount(cursor.rowcount)
 ccnnxx.close()
 self.tableData.setColumnCount(len(rows[0]))
 columnNames = [a[0] for a in cursor.description]
 g = 0
 h = 0
 i = 0
 j = 0
 k = 0
 counter = 0
 for items in columnNames:
 item = QtWidgets.QTableWidgetItem()
 self.tableData.setHorizontalHeaderItem(g, item)
 self.tableData.horizontalHeaderItem(g).setText(QtWidgets.QApplication.translate("MainWindow", columnNames[h], None, -1))
 header.setSectionResizeMode(g, QtWidgets.QHeaderView.ResizeToContents)
 g += 1
 h += 1
 for row in rows:
 counter += 1
 if i < counter:
 while j != len(rows[0]):
 item = QtWidgets.QTableWidgetItem()
 self.tableData.setItem(i, k, item)
 self.tableData.item(i, k).setText(QtWidgets.QApplication.translate("MainWindow", str(row[j]), None, -1))
 k +=1
 j +=1
 else:
 i +=1
 k = 0
 j = 0
 else:
 k = 0
 j = 0
 except IndexError:
 #print('This table has no information!')
 pass
 except TypeError:
 #double right-click protection
 pass
 else:
 pass
toolic
14.6k5 gold badges29 silver badges204 bronze badges
asked Sep 18, 2019 at 20:57
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

compare using is

 if getItem() != None:

Prefer if getItem() is not None:. It's just a weird python culture thing -- we examine the address of this singleton (id(None)) rather than examining equality.

Also PEP 8 explains that rather than LoadTable you meant to def load_table():. Similarly for get_item and self.table_data.

SELECT *

This is ambiguous:

 cursor.execute("SELECT * FROM " + var[1])

A maintenance engineer cannot tell what column a[0] will produce. Prefer SELECT a, b, c FROM ...

enumerate()

 for items in columnNames:
 ...
 h += 1

Prefer for h, items in enumerate(column_names):

Similarly with for counter, row in enumerate(rows):

no-op

Not sure what's up with suppressing IndexError and TypeError. But definitely this is pointless:

 else:
 pass
answered Sep 6 at 17:30
\$\endgroup\$
1
\$\begingroup\$

Simpler

There is no need for this else:

else:
 pass

Both lines can be deleted.

It looks like j and k are always assigned the same value. One of the variables shold be deleted.

Using while/else is uncommon, especially without a break. It is simpler without the else:

while j != len(rows[0]):
 item = QtWidgets.QTableWidgetItem()
 self.tableData.setItem(i, k, item)
 self.tableData.item(i, k).setText(QtWidgets.QApplication.translate("MainWindow", str(row[j]), None, -1))
 k +=1
 j +=1
i +=1
k = 0
j = 0

Comments

Delete commented-out code to reduce clutter:

#print('This table has no information!')

Naming

I think all these variables are used as counters (not just the one named counter).

g = 0
h = 0
i = 0
j = 0
k = 0
counter = 0

Your question states "forgot what i, j, k do". It would be a good idea to give all these variables more meaningful names.

DRY

This expression appears a couple of times:

len(rows[0])

It would be good to set it to a variable to reduce repetition. It is also more efficient because len will only be executed once.

Documentation

The PEP 8 style guide recommends adding docstrings for classes and functions.

try/except

The except statement is many lines away from the try line. PEP 8 recommends that you limit the try clause to the absolute minimum amount of code necessary to avoid masking bugs. It is hard to keep track of what line (or lines) are expected to result in the exception.

answered Sep 3 at 10:35
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.