3
\$\begingroup\$

I have a lot to learn in Python, and now I am adventuring in automating tasks with Selenium. Specifically, to just quickly explain the purpose of the script I have created, I have made a script that books a room at my school that I use to study in. Rather than doing this manually I figured this could easily have been done by a script.

My script works, and I use it with windows run scheduler so that it starts automatically, but it is awfully written. The problem is I don't know what I should improve, so I'm coming here hoping that someone can help me on what I can change in my code to make it better.

Ideas that I have myself:

  • I would really like to remove my time.sleeps in the code, so that python recognizes when the request is completed and then continues to the next step in the code rather than waiting for a fixed amount of time.

All advice is appreciated! Here is my code:

from selenium import webdriver
from selenium.webdriver.common.keys import Keys
import time
import re
import smtplib
import os
from email.mime.text import MIMEText
from email.mime.image import MIMEImage
from email.mime.multipart import MIMEMultipart
import datetime
from datetime import timedelta, date
import calendar

#You can skip this part and read from the function
now = datetime.datetime.now()
day_name = now.strftime("%A")

l = {"Monday" : 1, "Tuesday" : 2, "Wednesday" : 3, "Thursday" : 4, "Friday" : 5, "Saturday" : 6, "Sunday" : 7}
n = l[day_name]


Date = date.today()
EndDate = Date + timedelta(days=10)

print(Date)

date_DD = str(Date)[8:11]
date_DD_even = int(date_DD) % 2

def login():
    options = webdriver.ChromeOptions()
    options.add_argument("--start-maximized")

    browser = webdriver.Chrome("chromedriver directory", chrome_options = options)
    browser.get('website')

    time.sleep(1)

    navigate = browser.find_element_by_css_selector('#logincontrol > a.items.itemsbox')
    navigate.click()

    time.sleep(1)

    #user + password
    if date_DD_even == 0:
        user = browser.find_element_by_css_selector('#userNameInput')
        user.send_keys('user')
        password = browser.find_element_by_css_selector('#passwordInput')
        password.send_keys('password')

    time.sleep(3)

    login = browser.find_element_by_css_selector('#submitButton')
    login.click()

    navigate = browser.find_element_by_css_selector('#contents > div.linklist > div > div > a > div > span.lightlink > span')
    navigate.click()

    time.sleep(1)


    ###Choose date to book
    date_to_book = browser.find_element_by_css_selector('#leftresdate')
    text_area = browser.find_element_by_css_selector('#leftresdate').clear()
    date_to_book.send_keys(str(EndDate))
    date_to_book.send_keys(Keys.RETURN)

    time.sleep(3)

    #Open Schedule
    navigate = browser.find_element_by_css_selector('#objectselectionresult > table > tbody > tr.resetObjectExpand.eo13348.clickable > td.expname.nw')
    navigate.click()

    time.sleep(3)


    #Choose day to book
    if (n != 1 and n != 2):
        browser.execute_script("window.scrollTo(0, document.body.scrollHeight);")

        day_to_book = (n+5)%7
        time.sleep(2)
        if (day_to_book == 1):
            navigate = browser.find_element_by_css_selector('#objectselectionresult > table > tbody > tr:nth-child(5) > td > div.weekContainer > div:nth-child(1) > div.weekDiv > div.slotfree2.slotfreetarget')
            navigate.click()
        if (day_to_book == 2):
            navigate = browser.find_element_by_css_selector('#objectselectionresult > table > tbody > tr:nth-child(5) > td > div.weekContainer > div:nth-child(2) > div.weekDiv > div.slotfree2.slotfreetarget')
            navigate.click()
        if (day_to_book == 3):
            navigate = browser.find_element_by_css_selector('#objectselectionresult > table > tbody > tr:nth-child(5) > td > div.weekContainer > div:nth-child(3) > div.weekDiv > div.slotfree2.slotfreetarget')
            navigate.click()
        if (day_to_book == 4):
            navigate = browser.find_element_by_css_selector('#objectselectionresult > table > tbody > tr:nth-child(5) > td > div.weekContainer > div:nth-child(4) > div.weekDiv > div.slotfree2.slotfreetarget')
            navigate.click()                                
        if (day_to_book == 5):
            navigate = browser.find_element_by_css_selector('#objectselectionresult > table > tbody > tr:nth-child(5) > td > div.weekContainer > div:nth-child(5) > div.weekDiv > div.slotfree2.slotfreetarget')
            navigate.click()
        if (day_to_book == 6):
            pass
        if (day_to_book == 7):
            pass
    else:
        browser.quit()


    #Choose time to book
    time.sleep(3)
    navigate = browser.find_element_by_css_selector('#newResTimeDiv > tbody > tr > td.first > select.ui-widget.ui-state-default.timedrop.timeHourStart2 > option:nth-child(11)')
    navigate.click()
    time.sleep(2)
    navigate = browser.find_element_by_css_selector('#newResTimeDiv > tbody > tr > td.first > select.ui-widget.ui-state-default.timedrop.timeMinuteStart2 > option:nth-child(1)')
    navigate.click()
    time.sleep(2)
    navigate = browser.find_element_by_css_selector('#newResTimeDiv > tbody > tr > td.second > select.ui-widget.ui-state-default.timedrop.timeHourEnd2 > option:nth-child(18)')
    navigate.click()
    time.sleep(2)
    navigate = browser.find_element_by_css_selector('#newResTimeDiv > tbody > tr > td.second > select.ui-widget.ui-state-default.timedrop.timeMinuteEnd2 > option:nth-child(3)')
    navigate.click()

    #Press OK after choosing times

    time.sleep(2)
    navigate = browser.find_element_by_css_selector('#newResTimeDiv > tbody > tr > td.third > input')
    navigate.click()

    #Details of booking
    time.sleep(3)
    navigate = browser.find_element_by_css_selector('#leftreswrap > div > table > tbody > tr:nth-child(2) > td > span > span')
    navigate.click()

    time.sleep(3)

    navigate = browser.find_element_by_css_selector('#info0 > div.infoboxtitle')
    navigate.click()

    time.sleep(3)

    #Press OK to book
    navigate = browser.find_element_by_css_selector('#continueRes2')
    navigate.click()

    time.sleep(2)

    text = browser.find_element_by_xpath('//*[@id="resmadetoptext"]').text
    print(text)
    text = MIMEText(text)
    time.sleep(2)

    msg = MIMEMultipart()
    msg['Subject'] = "subject"
    msg['From'] = "sender"
    msg.attach(text)

    mail = smtplib.SMTP("smtp.gmail.com", 587)
    mail.ehlo()
    mail.starttls()
    mail.ehlo()
    mail.login("email", "email-pass")
    mail.sendmail("sender", "recipient", msg.as_string())
    print("email sent")
    mail.close()
    browser.quit()

login()
\$\endgroup\$
0

1 Answer 1

Reset to default
3
\$\begingroup\$

I see a few problems right off the bat; others may present themselves after the first round of refactoring.

  1. You've got too much code in the login() function. Functions ought to be 20 lines or less so they can be perceived with one glance. Create multiple functions, each of which has one job per the single responsibility principle.

  2. Use a main guard so that login() is not called unconditionally upon loading the file. Just one extra line:

    if __name__ == "__main__":
    login()

    (though again, this main() function should consist almost entirely of calling other functions). This allows using the source as a library without having to copy and paste.

  3. Your imports are too broad. It's much preferable to use the from x import y,z syntax to avoid polluting the global namespace with everything from these libraries.

  4. This comment

    #You can skip this part and read from the function

    is completely inscrutable. What do you mean? Who can skip? How do they skip? What part is being skipped?

  5. The code for detecting the weekday should be replaced with a call to n = now.isoweekday() This makes day_name and l completely unnecessary.
  6. Of course n is a horrible name for the variable. variable names should describe what they're used for.
  7. Why do you have both now and Date that are the same thing? I don't thnk now even needs to be a datetime since you're only using it for the day of the week.
  8. Instead of date_DD = str(Date)[8:11] use Date.day. No sense converting to a str when the date already has what you need. This also removes the need for the int call.
  9. day_to_book = (n+5)%7 produces a number from 0 to 6 inclusive. Why are you testing for 7?
  10. All these if day_to_book clauses are similar. Don't repeat yourself. Look at the code that's common, put what's different in a data structure, and you can really improve things.
  11. Similarly for the find_element_by_css_selector and then click. Don't repeat the same code or exactly the same code.
  12. The email and password should be in a config file that gets read, the path to the credential file should be a param or from the environment somehow. Never put credentials in your code.
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.