2
\$\begingroup\$

My script looks for handshake files in an input folder to determine whether certain processes have completed. If it finds them, it will update the database with the current time stamp.

Things I am wondering:

  • My update_db function takes 5 parameters. I have heard that in that case there is probably a better way to do it. Would this be a good candidate for creating a class and using an OO approach?
  • I am eventually going to be using a datetime to compare against the handshake timestamps, would it be difficult to somehow pull the file dates off our Linux server instead of creating them in the script?
  • Finally, any inconsistencies, ways I can simplify/improve my code. I am a new programmer of only 6 months, and love to learn:)

Here's the code:

def main():

    # Get current time stamp
    now = datetime.now()
    formatted_dt = '{0}/{1}/{2}-{3}:{4}:{5}'.format(now.month,
                                                    now.day,
                                                    now.year,
                                                    now.hour,
                                                    now.minute,
                                                    now.second)                                         
    # Connect to db
    sqlite_file = '/bcs/lgnt/clientapp/csvbill/database/csv.db'
    csv_connection = sqlite3.connect(sqlite_file)
    c = csv_connection.cursor()

    file_type = ['auditv2', 'handshake']

    # Check for files that don't have first hshake
    c.execute('SELECT pivot_id FROM aud_qty WHERE hshake1 is NULL')
    piv_id_list = [str(x[0]) for x in c.fetchall()]
    missing_list = update_db('CSV1000', file_type[0], piv_id_list, c, formatted_dt)
    # hshake1_miss = check_hshake('CSV1000', file_type[0], piv_id_list)
    # print(hshake1_miss)
    csv_connection.commit()
    csv_connection.close()


def update_db(proj_id, audit_type, piv_id_list, db_cursor, time_stamp):

    # chdir('/prod/bcs/lgnp/input')
    # prod_list = listdir('/prod/bcs/lgnp/input/')
    # for testing
    chdir('/bcs/lgnt/input')
    prod_list = listdir('/bcs/lgnt/input/')
    hshake_list = []

    for i, e in enumerate(prod_list):
        prod_list[i] = prod_list[i].split('_') # Field 6 is auditv2, Field 7 is the Pivot ID
    # Get only the Pivot ID
    for i, e in enumerate(prod_list):
        try:
            if prod_list[i][0] == proj_id and prod_list[i][6] == audit_type:
                hshake_list.append(prod_list[i][7])
        except IndexError:
                pass
    # pdb.set_trace()
    for i, e in enumerate(hshake_list):
        if hshake_list[i] in piv_id_list:
            print(hshake_list[i])
            # pdb.set_trace()
            db_cursor.execute('UPDATE aud_qty SET hshake1 = ? WHERE pivot_id = ?',
                             (time_stamp, hshake_list[i]))
            piv_id_list.remove(hshake_list[i])
    print(piv_id_list)
    return piv_id_list
\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

Working with resources

Whenever possible, anything that must be closed after using, in this example the database connection, it's good to wrap it in a with ... as, like this:

with sqlite3.connect(sqlite_file) as conn:
    cursor = conn.cursor()
    # ...

This way, conn will be automatically closed when exiting the block, so it's impossible to forget to close it, therefore more robust.

Using enumerate

enumarate is really handy, but you're not using it to its full potential:

for i, e in enumerate(prod_list):
    prod_list[i] = prod_list[i].split('_')

Do you see the unused variable e? Since you have it, why not use it:

for index, item in enumerate(prod_list):
    prod_list[index] = item.split('_')

Naming

The posted code is filled with poor names, for example:

  • the main function
  • the c cursor variable -> cursor
  • the i loop variable for index -> index
  • the e loop variable for an element -> item

At the minimum, try to avoid single-letter names.

mm/dd/yyyy format

If at all possible, I'd recommend to use any other date format.

Comprehensive map of all countries in the world that use the MMDDYYYY format:

enter image description here

\$\endgroup\$
2
\$\begingroup\$

Python actually has a date formatting function in datetime that makes it easier to do this than needing the usual string format method

now = datetime.now()
formatted_dt = '{0}/{1}/{2}-{3}:{4}:{5}'.format(now.month,
                                                now.day,
                                                now.year,
                                                now.hour,
                                                now.minute,
                                                now.second)

Can just be

timestamp = datetime.now().strftime('%m/%d/%y-%H:%M:%S')

See the full table about the method here. You can do some really helpful stuff, like automatically getting am/pm and getting data in the user's locale format. Also you'll note I changed the name, I think naming it for its purpose is clearly and removes the need for the comment too.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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