5
\$\begingroup\$

I would like feedback on the class I've written. The purpose is to dynamically create and interact with SQLite3 databases, accepting lists of complete or incomplete statements.

import sqlite3

class DB(object):    
    """DB initializes and manipulates SQLite3 databases."""

    def __init__(self, database='database.db', statements=[]):
        """Initialize a new or connect to an existing database.

        Accept setup statements to be executed.
        """

        #the database filename
        self.database = database
        #holds incomplete statements
        self.statement = ''
        #indicates if selected data is to be returned or printed
        self.display = False

        self.connect()

        #execute setup satements
        self.execute(statements)

        self.close()            

    def connect(self):
        """Connect to the SQLite3 database."""

        self.connection = sqlite3.connect(self.database)
        self.cursor = self.connection.cursor()
        self.connected = True
        self.statement = ''

    def close(self): 
        """Close the SQLite3 database."""

        self.connection.commit()
        self.connection.close()
        self.connected = False

    def incomplete(self, statement):
        """Concatenate clauses until a complete statement is made."""

        self.statement += statement
        if self.statement.count(';') > 1:
            print ('An error has occurerd: ' +
                'You may only execute one statement at a time.')
            print 'For the statement: %s' % self.statement
            self.statement = ''
        if sqlite3.complete_statement(self.statement):
            #the statement is not incomplete, it's complete
            return False
        else:
            #the statement is incomplete
            return True

    def execute(self, statements):
        """Execute complete SQL statements.

        Incomplete statements are concatenated to self.statement until they 
        are complete.

        Selected data is returned as a list of query results. Example: 

        for result in db.execute(queries):
            for row in result:
                print row
        """

        queries = []
        close = False
        if not self.connected:
            #open a previously closed connection
            self.connect()
            #mark the connection to be closed once complete
            close = True
        if type(statements) == str:
            #all statements must be in a list
            statements = [statements]
        for statement in statements:
            if self.incomplete(statement):
                #the statement is incomplete
                continue
            #the statement is complete
            try:
                statement = self.statement.strip()
                #reset the test statement
                self.statement = ''
                self.cursor.execute(statement)
                #retrieve selected data
                data = self.cursor.fetchall()
                if statement.upper().startswith('SELECT'):
                    #append query results
                    queries.append(data)

            except sqlite3.Error as error:
                print 'An error occurred:', error.args[0]
                print 'For the statement:', statement

        #only close the connection if opened in this function
        if close:
            self.close()   
        #print results for all queries
        if self.display:      
            for result in queries:
                if result:
                    for row in result:
                        print row
                else:
                    print result
        #return results for all queries
        else:
            return queries

    def terminal(self):
        """A simple SQLite3 terminal.

        The terminal will concatenate incomplete statements until they are 
        complete.        
        """

        self.connect()
        self.display = True

        print ('SQLite3 terminal for %s. Press enter for commands.' %
            self.database)

        while True:
            statement = raw_input('')
            if statement == '':
                user = raw_input(
                    'Type discard, exit (commit), or press enter (commit): ')
                if not user:
                    self.connection.commit()
                elif user == 'discard':
                    self.connect()
                elif user == 'exit':
                    break
            self.execute(statement)

        self.display = False
        self.close()

if __name__ == '__main__':     
    statement = ('CREATE TABLE %s (id INTEGER, filename TEXT);')                    
    tables = ['source', 'query']

    database = 'io.db'
    statements = [statement % table for table in tables]

    #setup
    db = DB(database, statements)

    #a single statement
    db.execute(
        ["INSERT INTO source (id, filename) values (8, 'reference.txt');"])

    #a list of complete statements
    db.execute(["INSERT INTO query (id, filename) values (8, 'one.txt');",
                "INSERT INTO query (id, filename) values (9, 'two.txt');"])

    #a list of incomplete statements
    db.execute(["INSERT INTO query (id, filename) ", 
                "values (10, 'three.txt');"])

    #retrieving multiple query results
    queries = ['SELECT * FROM source;', 'SELECT * FROM query;']
    for result in db.execute(queries):
        print result

[(8, u'reference.txt')]
[(8, u'one.txt'), (9, u'two.txt'), (10, u'three.txt')]
\$\endgroup\$
  • \$\begingroup\$ This looks rather interesting, but also rather hard-coded. Did you by chance figure out a method of creating tables in a more generic manner? \$\endgroup\$ – Mast Dec 5 '16 at 10:33
  • \$\begingroup\$ @Mast I've been trying to brainstorm a more generic way. One idea was to not allow incomplete statements. In the big picture, no program can do everything. Maybe it's better to focus on accepting only good input. Also, In the future I'd like to completely omit print statements and terminals in favor of logging. \$\endgroup\$ – 12345678910111213 Dec 7 '16 at 9:58
2
\$\begingroup\$

Overall this looks good. Documentation and comments look pretty good, and the code is fairly sensible. Some comments:

  • Beware of mutable default arguments! In particular, statements=[]. If the statements variable gets mutated within the class, that persists through all instances of the class. Better to have a default value of None, and compare to that, i.e.:

    def __init__(self, database='database.db', statements=None):
        if statements is None:
            statements = []
    

    Although perhaps it would be better to defer this checking to the point where you’re about to connect and run the command – if there are no commands to execute (whether the user skipped that argument, or supplied an empty list) – you could skip setting up and closing the DB connection entirely.

  • It’s common to print errors to stderr, so I’d modify the print() on line 46–7 accordingly. Also, you’ve misspelt occurred.

  • It’s better to use exceptions to indicate control flow, not just printing errors. This allows to caller to handle them accordingly. Within DB.incomplete(), I’d consider throwing a ValueError if the user passes multiple statements. Likewise lines 96–8 in DB.execute().

  • In DB.incomplete(), you can simplify the return statement:

    return not sqlite3.complete_statement(self.statement)
    

    This is both simpler and more Pythonic.

  • PEP 8 convention is that comments start by a hash following by a space, not a hash and then straight into prose.

  • There’s use of print as a statement and as a function. You should be consistent – I’d recommend going to the print function, and adding

    from __future__ import print_function
    

    to the top of the file. This is future-proofing for Python 3.

  • Because you have separate connect() and close() methods, it’s possible that somebody could call connect(), execute some statements, and then hit an exception before they could close(). This means the database connection could stick around for longer than you were expecting.

    You might want to consider defining a context manager for your class. This allows people to use with statement and your class, similar to the with open(file) construction:

    with DB(database='chat.db') as mydb:
        # do stuff with mydb
    

    And then the cleanup code on the context manager always runs, even if you hit an exception in the body of the with statement.

  • I don’t know what the two lists defined at the end of the file are. Left-over cleanup code?

\$\endgroup\$
  • \$\begingroup\$ The 2 lists at the end are the printed query results returned as table rows. Thanks for the feedback \$\endgroup\$ – 12345678910111213 Jul 18 '16 at 0:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.