Flagging employees whose spending is close to the budgeted amount

Question

I am working with an SQLite database. Is this code written the best way it can be and correct? The code:

1. Selects from the db adding a column for the percentage difference between two columns

2. Loops through the values of the columns

3. If the date is in the past

4. If the price is greater than 500 and the percentage difference is less than 1st argument set flag to 1

5. Else if the price is less than 500 and the percentage difference is less than 2nd argument set flag to 1

6. Otherwise keep the flag as 0

from datetime import datetime as dt

def calculateEmployeeSpend(read_cursor, flag_higher_amount, flag_lower_budget):

    read_cursor.execute("SELECT distinct b.employee_id, b.amount, "
                "s.spend, b.date, b.amount - s.spend as spend_left,  "
                "100.0*(b.amount - s.spend) / b.amount As PercentDiff FROM employee_budget_upload "
                "As b JOIN employee_budget_spent As s ON  b.employee_id = s.employee_id where b.amount != 0")

    for employee_id, amount, spend, date, spend_left, percent_diff in read_cursor:
        flag=0
        date_of_amount = dt.strptime(date, "%d/%m/%Y")
        if date_of_amount <= dt.now():
            if amount > 500 and percent_diff < int(flag_higher_amount) or amount < 500 and percent_diff < int(flag_lower_budget):
                flag=1

        write_cursor.execute('INSERT OR REPLACE INTO employee_overall_spend VALUES (?,?,?,?,?,?,?,?)',
                         (employee_id, amount, spend, date, spend_left, percent_diff, dt.now().strftime("%d/%m/%Y"), flag)) 

Answer 1

I would say being a small code, you can change a couple things to make it a bit more readable

Specially the part with the conditionals, is a bit hard to follow

date_of_amount = dt.strptime(date, "%d/%m/%Y")
if date_of_amount <= dt.now():
    if amount > 500 and percent_diff < int(flag_higher_amount) or amount < 500 and percent_diff < int(flag_lower_budget):
        flag=1

You could try extracting all into smaller functions:

is_past_amount_date = dt.strptime(date, "%d/%m/%Y") <= dt.now()
should_flag_amount = (amount > 500 and percent_diff < int(flag_higher_amount)) or (amount < 500 and percent_diff < int(flag_lower_budget))

if is_past_amount_date and should_flag_amount:
    flag = 1

As well, try not to use date as a variable number since it shadows the date name from datetime, it can have some undesired effects later

To end with it, the execution of the cursor can take place outside the function, so as a first parameter you receive employee_budget_cursors (which is the result of executing the select Then, you iterate with it in your function directly

Also easier for testing

Answer 2

There's two ways you can go about this. Stick with your current way, or use SQL more.

Current way

• I'd change your for loop to assign to one value.
• I'd make a function that tells you if the values have the flag.
• I'd use *value to tuple unpack into the write_cursor tuple.

And so would get something like:

from datetime import datetime as dt


def get_flag(employee_spend, higher, lower):
    _, amount, _, date, _, percent_diff = employee_spend
    return int(
        dt.strptime(date, "%d/%m/%Y") <= dt.now()
        and (
            amount > 500
            and percent_diff < int(higher)
            or amount < 500
            and percent_diff < int(lower)
        )
    )


def calculateEmployeeSpend(read_cursor, flag_higher_amount, flag_lower_budget):
    higher, lower = int(flag_higher_amount), int(flag_lower_budget)

    read_cursor.execute("SELECT distinct b.employee_id, b.amount, "
                "s.spend, b.date, b.amount - s.spend as spend_left,  "
                "100.0*(b.amount - s.spend) / b.amount As PercentDiff FROM employee_budget_upload "
                "As b JOIN employee_budget_spent As s ON  b.employee_id = s.employee_id where b.amount != 0")

    for employee_spend in read_cursor:
        flag = get_flag(employee_spend, higher, lower)
        write_cursor.execute(
            'INSERT OR REPLACE INTO employee_overall_spend VALUES (?,?,?,?,?,?,?,?)',
            (*employee_spend, dt.now().strftime("%d/%m/%Y"), flag)
        )

Use SQL

• If you chose to use SQL I'd change it so that you pass higher and lower to the SQL query.
• I'd move the flag logic into the SQL query.
• I'd move the current time logic into the SQL query. I found how to get the current time from here.
• I'd look into moving the SQL commands into their own files, so that they're easier to maintain.

I'm don't know SQL, but you could use something like:

CALCULATE_EMPLOYEE_SPEND_READ = '''\
DECLARE @curr_date DATETIME;
SET @curr_date = GETDATE();

SELECT
    e.employee_id,
    e.amount,
    e.spend,
    e.date,
    e.spend_left,
    e.percent_diff,
    @curr_date as curr_date,
    case when
        e.date <= @curr_date
        and (
            e.amount > 500
            and e.percent_diff < ?
            or e.amount < 500
            and e.percent_diff < ?
        )
        then 1
        else 0 
    end
    as flag
FROM (
    SELECT distinct
        b.employee_id,
        b.amount,
        s.spend,
        b.date,
        b.amount - s.spend as spend_left,
        100.0 - 100.0*s.spend / b.amount as percent_diff
    FROM employee_budget_upload As b
    JOIN employee_budget_spent As s
    ON  b.employee_id = s.employee_id
    WHERE b.amount != 0
) as e
'''
CALCULATE_EMPLOYEE_SPEND_WRITE = 'INSERT OR REPLACE INTO employee_overall_spend VALUES (?,?,?,?,?,?,?,?)'


def calculateEmployeeSpend(read_cursor, higher, lower):
    read_cursor.execute(CALCULATE_EMPLOYEE_SPEND_READ, (int(higher), int(lower)))
    for employee_spend in read_cursor:
        write_cursor.execute(CALCULATE_EMPLOYEE_SPEND_WRITE, employee_spend)