Skip to main content
Code Review

Return to Answer

added 1 character in body
Source Link
Konrad Rudolph
  • 6.7k
  • 23
  • 35
import sqlite3
import time
TEST_NUM_ROWS = 200
TEST2_INC = 200
TEST3_INC = 400
def database_test():
 with conn = sqlite3.connect('SQLite_Test.db') as conn:
 for i in range(TEST_NUM_ROWS):
 row = conn.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))
 row = c).fetchone()
 new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
 conn.execute('UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?', new_row + (i,))
start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time), 4))
import sqlite3
import time
TEST_NUM_ROWS = 200
TEST2_INC = 200
TEST3_INC = 400
def database_test():
 with conn = sqlite3.connect('SQLite_Test.db'):
 for i in range(TEST_NUM_ROWS):
 conn.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))
 row = c.fetchone()
 new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
 conn.execute('UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?', new_row + (i,))
start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time), 4))
import sqlite3
import time
TEST_NUM_ROWS = 200
TEST2_INC = 200
TEST3_INC = 400
def database_test():
 with sqlite3.connect('SQLite_Test.db') as conn:
 for i in range(TEST_NUM_ROWS):
 row = conn.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,)
 ).fetchone()
 new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
 conn.execute('UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?', new_row + (i,))
start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time), 4))
Source Link
Konrad Rudolph
  • 6.7k
  • 23
  • 35

Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.

First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:

 # Open Database

Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.

Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.

 c = conn.cursor()

Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.

 i = 0

This initialisation is redundant: range(...) does it for you.

 for i in range(200):

Why 200? Avoid hard-coding "magic" numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.

 DB_Values = [] 
 DB_Values = c.fetchone()

The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.

Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.

 Value1 = DB_Values[0]+200
 Value2 = DB_Values[1]+400

Same comment regarding names, and same comment regarding magic numbers.

 c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
 i += 1 

You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.

 # Save (commit) the changes

Same as above: this comment isn’t helpful, since it just repeats what the code says.

 conn.commit()

If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.


Taking this in account, here’s a rewritten version of your code:

import sqlite3
import time
TEST_NUM_ROWS = 200
TEST2_INC = 200
TEST3_INC = 400
def database_test():
 with conn = sqlite3.connect('SQLite_Test.db'):
 for i in range(TEST_NUM_ROWS):
 conn.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?', (i,))
 row = c.fetchone()
 new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
 conn.execute('UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?', new_row + (i,))
start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time), 4))
lang-py

AltStyle によって変換されたページ (->オリジナル) /