1
\$\begingroup\$

I have been trying to improve my Python coding and was just wondering if there was a more pythonic way to perform what I have done so far, namely around the date in the main() function.

import datetime
import sybase

account_list = []

def get_date(): # Date -1 day unless Monday then -3 days
    d = datetime.date.today()
    if d.weekday() == 0:
        date = d - datetime.timedelta(days=3)
        return date
    else:
        date = d - datetime.timedelta(days=1)
        return date


def get_unique_accounts(date):
    conn = sybase.Connection('')
    c = conn.cursor()
    c.execute("SELECT DISTINCT acct from accounts WHERE date = @date", {"@date": date})
    result = c.fetchall()
    for row in result:
        account_list.append(row[0])
    c.close()
    conn.close()


def check_balance(date):
    conn = sybase.Connection('')
    c = conn.cursor()
    c.execute("SELECT TOP 1 name,balance from balance WHERE date = @date", {"@date": date})
    result = c.fetchone()
    c.close()
    conn.close()
    return result


def main():
    date = get_date()
    get_unique_accounts(date) # This will be used for future functions
    check_balance(date)
    

if __name__ == "__main__":
    main()
\$\endgroup\$
5
  • \$\begingroup\$ Sybase, though? Why? \$\endgroup\$ – Reinderien Mar 30 at 17:41
  • \$\begingroup\$ I have no control over the current in place DB solution :( \$\endgroup\$ – SimonT Mar 30 at 17:43
  • \$\begingroup\$ That's deeply unfortunate. For your own learning purposes, if you have the opportunity I suggest trying out PostgreSQL instead of a museum piece. \$\endgroup\$ – Reinderien Mar 30 at 17:58
  • \$\begingroup\$ You've tagged this as both Python 2 and 3. What are the constraints? Does the sybase package work with 3? \$\endgroup\$ – Reinderien Mar 30 at 17:59
  • \$\begingroup\$ I will do for sure, as I've used MySQL and variants when I did PHP coding. But as for what I've posted above is there a more 'pythonic' method? Or have I done all I can so far correctly. We have a modified sybase module that we can import for both 2 and 3. \$\endgroup\$ – SimonT Mar 30 at 18:00
2
\$\begingroup\$

I will focus on the get_date() method. First of all, what sort of date is it? Currently the name makes me think it will get the current date, but it's sort of a banking date? Something along the lines of get_last_banking_day() would be more descriptive?

You can move the return to the end of the function as that is the same in both blocks of the if/else:

def get_last_banking_day(): # Date -1 day unless Monday then -3 days
    d = datetime.date.today()

    if d.weekday() == 0:
        date = d - datetime.timedelta(days=3)
    else:
        date = d - datetime.timedelta(days=1)
    
    return date

You could clean up the import and only import the 2 things needed from datetime, date and timedelta.:

from datetime import date, timedelta

The function would become:

def get_last_banking_day(): # Date -1 day unless Monday then -3 days
    d = date.today()

    if d.weekday() == 0:
        date = d - timedelta(days=3)
    else:
        date = d - timedelta(days=1)
    
    return date

Now we can look at variable names and do some cleanup, especially the d variable. Always try to avoid non-descriptive names:

def get_last_banking_day(): # Date -1 day unless Monday then -3 days
    today = date.today()

    if today.weekday() == 0:
        last_banking_day = today - timedelta(days=3)
    else:
        last_banking_day = today - timedelta(days=1)
    
    return last_banking_day

As the today - timedelta(days=1) is the default we use it as such and set the value to this and only use an if-statement to set the exception:

def get_last_banking_day(): # Date -1 day unless Monday then -3 days
    today = date.today()
    last_banking_day = today - timedelta(days=1)

    if today.weekday() == 0:
        last_banking_day = today - timedelta(days=3)

    return last_banking_day
\$\endgroup\$
3
  • \$\begingroup\$ AH thanks Korfoo, the if loop where there is already a default is mistake I need to remove from my mindset when coding. Although just for reference you still have 'return date' rather than 'return 'last_banking_day' \$\endgroup\$ – SimonT Apr 1 at 6:32
  • \$\begingroup\$ Oops, I edited it to correct the return. Do note that I only looked at the get_date method as I do not have experience with sybase so I can not give tips about that :) \$\endgroup\$ – Korfoo Apr 1 at 7:03
  • \$\begingroup\$ Appreciate it, I do not think there are improvements that could be made elsewhere in what has been posted. \$\endgroup\$ – SimonT Apr 1 at 8:12

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.