4
\$\begingroup\$

I am trying to learn best practices for OOP and unit testing. I have implemented a simple User class here which checks if the user exists in the database. If yes, then it loads the user data into the instance. If not, then it should create a new entry in the database.

My questions:

  1. How should I create unit tests for the methods? For example, for check_user_exists() should I mock for conn or cursor or exists or three of them? I tried to research for this but none of the tutorials for mocking in Python really gave me any true understanding of how good mocking is done.
  2. In each of the methods, all the database connection related codes are repeated (i.e. conn.cursor(), conn.close()). Can these be refactored away? My guess would be to create a Database class, but I'm not sure about that.
  3. Is it weird for User class to have a database_path attribute? So far this is the best that I have come up with. Please give any suggestions on how to structure a class where it has to always query databases.

user.py

import sqlite3


class User:
    def __init__(self, username, database_path):
        self.username = username
        self.database_path = database_path

        if not self.check_user_exists():
            self.create_user()
        else:
            self.get_user()

    def check_user_exists(self):
        conn = sqlite3.connect(self.database_path)
        cursor = conn.cursor()

        cursor.execute('SELECT rowid FROM user WHERE name = ?', (self.username,))
        exists = cursor.fetchone()

        conn.close()

        if exists is None:
            return False
        else:
            return True

    def create_user(self):
        conn = sqlite3.connect(self.database_path)
        cursor = conn.cursor()

        cursor.execute("INSERT INTO user (name) VALUES (?)", (self.username,))

        conn.commit()
        conn.close()

    def get_user(self):
        conn = sqlite3.connect(self.database_path)
        cursor = conn.cursor()

        cursor.execute("SELECT data FROM user WHERE name=?", (self.username,))
        self.data = cursor.fetchone()[0]

        conn.close()
\$\endgroup\$
  • \$\begingroup\$ What does "OOP" stand for? \$\endgroup\$ – Iam Pyre Jan 13 '18 at 18:13
  • 1
    \$\begingroup\$ @IamPyre It's Object Oriented Programming \$\endgroup\$ – yanxun Jan 13 '18 at 18:19
4
\$\begingroup\$

This is an interesting question. Thanks for posting it!

IMO, you don't actually have a User class here. A User would be a system object that has a name, maybe an id, a password, some relationships to material, perhaps one or more privilege levels.

What you have here is an object that provides an interface to a database table - specifically, the user table. This is a Table Data Gateway in the parlance of Martin Fowler.

Let's write some sample code:

from private_data import database_path

username = 'yanxun'
user = User(username, database_path)

if user.check_user_exists():
    print("User {} is in the database".format(user.username))

That looks pretty clunky. If you rename your class to UserTable, it starts to make more sense:

from private_data import database_path

username = 'yanxun'
users = UserTable(database_path)

if username in users:      # implemented by .__contains__ method
    print("User {} is in the database.".format(username))
else:
    print("User {} not found. Adding to the database.".format(username))

    users.add(username)

Note that I am treating the table as a collection, specifically, like a set, and presuming you write method names that are inspired by existing Python collection names.

Now, with that out of the way, let me propose that you should be lazy in how you handle creating and deleting database connections. Specifically, make the caller do it!

Making the caller pass you in a connection is the best way to write code that you want to test, as well as the best way to write code that is maximally portable. If the caller mocks a database, you don't know and don't care. If the caller connects you to Oracle instead of sqlite, you don't know and don't care (provided your SQL syntax is valid).

def __init__(self, dbconn):
    self.dbconn = dbconn

Now you don't have to worry about database username/password, database path, or anything else. You can just request a cursor from the connection, and do your thing.

(Note: transaction support, and two-phase-commit support, are connection level operations, rather than cursor operations. That's why I suggest passing in a connection, rather than a cursor. See PEP-249 for details.

\$\endgroup\$
  • \$\begingroup\$ You might want to be explicit and add that for the if username in users line to work, the magic method __contains__ should be defined. Otherwise very nice answer. \$\endgroup\$ – Graipher Jan 14 '18 at 0:48
  • \$\begingroup\$ Thanks for the reply! This is definitely a more sensible approach than mine. So if I implemented the UserTable instead, I only need to test methods for (by mocking) the results of the sql query based on inputs, is that right? Is this way (using UserTable) comparable to using ORM such as SQLAlchemy? \$\endgroup\$ – yanxun Jan 14 '18 at 14:28
  • \$\begingroup\$ Martin Fowler's book, Patterns of Enterprise Application Architecture goes into much more detail on this and other patterns. You can find them discussed a lot on the web. Since the point of the pattern is to encapsulate the SQL in common operations, your testing approach should be to create a simple database, then run the various methods on it. (Create table with user x, but no user y. Run "findUser(x)" and "findUser(y)" and confirm one passes the other fails, addUser(x) fails, etc.) \$\endgroup\$ – Austin Hastings Jan 14 '18 at 18:27
  • \$\begingroup\$ TableDataGateway is a much simpler pattern than ObjectRelationalModel (ORM). In most cases, simpler means faster, too. I'd suggest taking a hard look at what you really need to get done, and then seeing if TDGW is enough pattern for the job- it usually is- before trying to add something like an ORM tool to your code. \$\endgroup\$ – Austin Hastings Jan 14 '18 at 18:30

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.