6
\$\begingroup\$

I just came back to programming and decided to create a car leasing system (GitHub page here.) to teach myself about databases, gui programming and web development. And I would like some input on what I can imporve on a specific function that I just rewrote.

What the function does is add a car lease to the customers name, makes the car unavailable for others to lease and adds a record of the lease. All to the database.

I am pretty happy with it but I know there is parts where I can improve.

Here is the function:

import sqlite3
import datetime


conn = sqlite3.connect("db/test.db")
c = conn.cursor()
current_datetime = datetime.datetime.now()

def add_lease(customer_id, car_id, lease_expire):
  """
  Adds lease to customers account and 
  removes the car from available cars list.
  """

  lease_start = current_datetime.strftime("%Y-%m-%d")
  car_active = c.execute("SELECT IS_LEASED FROM CARS WHERE ID=?",
      (car_id,)).fetchone()[0]
  lease_active = c.execute("SELECT ACTIVE_LEASE FROM CUSTOMERS WHERE ID=?",
      (customer_id,)).fetchone()[0]

  if not car_active or car_active == None:
      if not lease_active or lease_active == None:
          c.execute("UPDATE CARS SET IS_LEASED = 1 WHERE ID = ?",
              (car_id,))
          c.execute("""INSERT INTO LEASES (CAR_ID, LEASE_START, LEASE_EXPIRE, CUSTOMER, IS_ACTIVE) 
              VALUES (?, ?, ?, ?, ?)""", (car_id, lease_start, lease_expire, customer_id, 1,))

          lease_id = c.execute("SELECT ID FROM LEASES").fetchall()
          lease_id = ''.join(c for c in lease_id if c not in '[](),')
          c.execute("UPDATE CUSTOMERS SET LEASE_ID = ? WHERE ID = ?",
              (lease_id, customer_id,))

          c.execute("UPDATE CUSTOMERS SET ACTIVE_LEASE = 1 WHERE ID = ?",
              (customer_id,))

          conn.commit()
  else:
      return 2

If you want to try it out yourself here is the db_setup.py file:

import sqlite3 as lite

con = lite.connect('db/test.db')

with con:
    cur = con.cursor()
    cur.execute("""CREATE TABLE CUSTOMERS(
        ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, 
        NAME TEXT, 
        ADDRESS CHAR(50), 
        EMAIL_ADDRESS CHAR(50),
        PHONE_NUMBER CHAR(50),
        LEASE_ID TEXT,
        ACTIVE_LEASE INT)
        """)
    cur.execute("""CREATE TABLE CARS(
        ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,  
        CAR TEXT, 
        CAR_COLOR TEXT, 
        CAR_LICENSE TEXT,
        LAST_LEASED_BY INT,
        IS_LEASED INT)""")
    cur.execute("""CREATE TABLE LEASES(
        ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
        CAR_ID INT, 
        LEASE_START TEXT, 
        LEASE_EXPIRE TEXT,
        CUSTOMER INT,
        IS_ACTIVE INT)""")
    cur.execute("""INSERT INTO CUSTOMERS (NAME, ADDRESS, EMAIL_ADDRESS, PHONE_NUMBER) VALUES 
        ("Rooney Paul","P.O. Box 569, 4862 Urna St.","at.velit@per.com","(01112) 129811"),
        ("Wallace Armstrong","7690 Est. Rd.","Nunc.lectus@dapibusquam.co.uk","070 7854 9488"),
        ("Erich Robertson","Ap #431-4682 Adipiscing St.","eu@morbi.ca","(012777) 33360"),
        ("Kennan Peterson","P.O. Box 438, 4390 Dapibus Road","Lorem@mollisvitae.org","0800 822615"),
        ("Brock Cantu","P.O. Box 223, 1483 Nunc St.","nec@Seddiam.net","055 0209 8442"),
        ("Fulton Buchanan","4090 Posuere Avenue","nunc@fringillaeuismodenim.co.uk","07624 328923"),
        ("Duncan Pruitt","P.O. Box 567, 6726 Et, Rd.","In.tincidunt.congue@nullavulputatedui.net","07624 343171"),
        ("Simon Horton","532-1265 Arcu Rd.","eros.non.enim@quis.ca","0800 398826"),
        ("Samuel Dunlap","905-1619 Felis. Av.","Vivamus.nibh@mollisPhaselluslibero.com","076 9655 0399"),
        ("James Rosa","530-7282 Tellus Av.","imperdiet.nec@liberolacus.org","055 3901 7569");
        """)
    cur.execute("""INSERT INTO CARS (CAR, CAR_COLOR, CAR_LICENSE) VALUES 
        ("BMW", "RED", "AAA 111"), 
        ("AUDI", "BLUE", "BBB 111"),
        ("MERCEDES", "BLACK", "CCC 111"),
        ("VOLVO", "WHITE", "CCC 111"),
        ("PORCHE", "YELLOW", "DDD 111"),
        ("FORD", "GREEN", "EEE 111"),
        ("DODGE", "GREY", "FFF 111"),
        ("HONDA", "DARK-RED", "GGG 111"),
        ("CADILLAC", "WHITE", "HHH 111"),
        ("ALFA ROMEO", "PURPLE", "JJJ 111")
        """)

I am happy to answer any questions you may have!

Thank you!

\$\endgroup\$
3
+50
\$\begingroup\$

I don't really like to keep db connection as global and always open

conn = sqlite3.connect("db/test.db")
c = conn.cursor()

Instead you could create a simple context manager that will help you with this, and will make your code prettier: from contextlib import contextmanager

@contextmanager
def get_cursor(db_name='db/test.db'):
    conn = sqlite3.connect(db_name)
    yield conn.cursor()
    try:
        conn.commit()
    finally:
        conn.close()

So now whenever you will need a cursor you can just do:

with get_cursor() as c:
    c.execute(...)
    c.execute(...)

It will commit and close the connection as soon as it will go out of with statement

Another issue here is having variable name c is not a best practice, better user a full name cursor so it will be easier for others to understand what is going on here.

Making current_datetime = datetime.datetime.now() global is also not a good choice, instead, you can just initialize it each time you call your add_lease

So in the end your code should look like this:

import datetime
import sqlite3
from contextlib import contextmanager


@contextmanager
def get_cursor(db_name='db/test.db'):
    conn = sqlite3.connect(db_name)
    yield conn.cursor()
    try:
        conn.commit()
    finally:
        conn.close()


def add_lease(customer_id, car_id, lease_expire):
    """
    Adds lease to customers account and
    removes the car from available cars list.
    """

    current_datetime = datetime.datetime.now()
    lease_start = current_datetime.strftime("%Y-%m-%d")
    with get_cursor() as cursor:
        car_active = cursor.execute("SELECT IS_LEASED FROM CARS WHERE ID=?",
                                    (car_id,)).fetchone()[0]
        lease_active = cursor.execute("SELECT ACTIVE_LEASE FROM CUSTOMERS WHERE ID=?",
                                      (customer_id,)).fetchone()[0]

    if not car_active and not lease_active:
        with get_cursor() as cursor:
            cursor.execute("UPDATE CARS SET IS_LEASED = 1 WHERE ID = ?",
                           (car_id,))
            cursor.execute("INSERT INTO LEASES (CAR_ID, LEASE_START, LEASE_EXPIRE, CUSTOMER, IS_ACTIVE) "
                           "VALUES (?, ?, ?, ?, ?)", (car_id, lease_start, lease_expire, customer_id, 1))

            lease_id = cursor.execute("SELECT ID FROM LEASES").fetchall()
            lease_id = ''.join(char for char in lease_id if char not in '[](),')
            cursor.execute("UPDATE CUSTOMERS SET LEASE_ID = ? WHERE ID = ?",
                           (lease_id, customer_id))

            cursor.execute("UPDATE CUSTOMERS SET ACTIVE_LEASE = 1 WHERE ID = ?",
                           (customer_id,))
    else:
        return 2

I didn't get what is this magic number 2 so I left this part untouched.

\$\endgroup\$
1
  • \$\begingroup\$ Enjoy your bounty :) \$\endgroup\$ – Ave Nov 26 '16 at 22:09

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.