HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Car leasing system - add lease to database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
carsystemleasedatabaseleasingadd

Problem

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

Solution

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.

Code Snippets

conn = sqlite3.connect("db/test.db")
c = conn.cursor()
@contextmanager
def get_cursor(db_name='db/test.db'):
    conn = sqlite3.connect(db_name)
    yield conn.cursor()
    try:
        conn.commit()
    finally:
        conn.close()
with get_cursor() as c:
    c.execute(...)
    c.execute(...)
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

Context

StackExchange Code Review Q#147139, answer score: 3

Revisions (0)

No revisions yet.