0
\$\begingroup\$

I'm a beginner developer in Python and I need some tips to improve my code, in performance (speed) and hardware usage. Is it possible to improve?

And I have a question: is it possible to keep the browser (chrome/chromedriver) open after execution?

The objective: to automate scheduling at the Italian Consulate, through the website prenot@mi. Booking on the Prenot@mi system is "simple", but it's very busy, so I have to be quicker than others to get a reservation. Within seconds, a booking is "lost" as another scheduler was able to book faster. Dates are available on Mondays and Wednesdays at 11:00 am.

My repository: https://github.com/skynorreply/prenot-mi_script

import datetime
import time
import os

from selenium import webdriver
from selenium.common.exceptions import TimeoutException
from selenium.webdriver.common.by import By
from selenium.webdriver.support import expected_conditions as EC
from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.common.alert import Alert

def esta_na_hora(hora, minuto, segundos, data_atual):
    if data_atual.hour == hora and data_atual.minute == minuto and data_atual.second == segundos:
        return True
    return False


def processa_dias_da_semana(dias_da_semana):
    dias_da_semana_int = []
    for dia in dias_da_semana:
        if dia == "seg":
            dias_da_semana_int.append(0)
        if dia == "ter":
            dias_da_semana_int.append(1)
        if dia == "qua":
            dias_da_semana_int.append(2)
        if dia == "qui":
            dias_da_semana_int.append(3)
        if dia == "sex":
            dias_da_semana_int.append(4)
        if dia == "sab":
            dias_da_semana_int.append(5)
        if dia == "dom":
            dias_da_semana_int.append(6)
    return dias_da_semana_int


def esta_no_dia_da_semana(dias_da_semana, data_atual):
    if data_atual.weekday() in dias_da_semana:
        return True

    return False

hora_string = input("Que horas quer agendar? (hh:mm:sg): ")

dia_da_semana_string = input(
    "Quais dias da semana? (seg ter qua qui sex sab dom): ")

hora = int(hora_string.split(':')[0])
minuto = int(hora_string.split(':')[1])
segundos = int(hora_string.split(':')[2])

dias_da_semana = dia_da_semana_string.split(' ')
dias_da_semana_int = processa_dias_da_semana(dias_da_semana)
ativo = True
while ativo:
    agora = datetime.datetime.now()
    print(agora)
    if esta_na_hora(hora, minuto, segundos, agora) and esta_no_dia_da_semana(dias_da_semana_int, agora):
        ativo = False
        chrome_options = webdriver.ChromeOptions() #here begins to configure chrome not to load the images
        prefs = {"profile.managed_default_content_settings.images": 2}
        chrome_options.add_experimental_option("prefs", prefs)
        navegador = webdriver.Chrome(chrome_options=chrome_options) #here finish configuring chrome to not load the images
        navegador.get("https://prenotami.esteri.it") #access the site
        time.sleep(3) #to wait the loading, only by safety
        navegador.find_element(
            By.ID, "login-email").send_keys("email@email.com") #REMEMBER TO PUT THE EMAIL
        navegador.find_element(
            By.ID, "login-password").send_keys("password") #REMEMBER TO PUT THE PASSWORD
        navegador.find_element(
            By.XPATH, '//*[@id="login-form"]/button').click() #will click on the button to login
        time.sleep(3) #this time is important!!! You run the risk of failing.
        navegador.find_element(By.ID, "advanced").click() #click on the book section
        time.sleep(2)
        navegador.find_element(
            By.XPATH, '//*[@id="dataTableServices"]/tbody/tr[2]/td[4]/a/button').click() #here the system will click on the option Reserve Citizenship
        time.sleep(2)
        i = 1
        while navegador.find_element(By.XPATH, '/html/body/div[2]/div[2]/div/div/div/div/div/div/div/div[4]/button') == navegador.find_element(By.XPATH, '/html/body/div[2]/div[2]/div/div/div/div/div/div/div/div[4]/button'):
            print("Tentativa Nº - " + str(i))
            navegador.find_element(
                By.XPATH, '/html/body/div[2]/div[2]/div/div/div/div/div/div/div/div[4]/button').click()
            delay = 10  #seconds
            try:
                elemento = WebDriverWait(navegador, delay).until(EC.presence_of_element_located(
                    (By.XPATH, '//*[@id="dataTableServices"]/tbody/tr[2]/td[4]/a/button')))
                elemento.click()
            except TimeoutException:
                print("elemento nao encontrado")
            i = i + 1
        time.sleep(1)
navegador.find_element(By.ID, 'File_0').send_keys(os.path.abspath("/Users/compCOPEL.pdf")) #proof of address
navegador.find_element(By.ID, 'PrivacyCheck').click() #accept the privacy term
navegador.find_element(By.ID, 'btnAvanti').click() #forward button to calendar view
alert = Alert(navegador) #here is to identify the window that opens to confirm the appointment
alert.accept() #here will accept confirmation window

Thank you in advance!

\$\endgroup\$
4
  • \$\begingroup\$ It's all in Italian. This could greatly reduce the likelihood of someone writing an answer. \$\endgroup\$
    – Mast
    Nov 19 at 12:24
  • \$\begingroup\$ @Mast Ok, sorry. I translated to English!!! Please, help me!! \$\endgroup\$
    – user267072
    Nov 19 at 12:52
  • 1
    \$\begingroup\$ Please don't make more work for other people by vandalizing your posts. By posting on the Stack Exchange network, you've granted a non-revocable right, under the CC BY-SA 4.0 license, for Stack Exchange to distribute that content (i.e. regardless of your future choices). By Stack Exchange policy, the non-vandalized version of the post is the one which is distributed. Thus, any vandalism will be reverted. If you want to know more about deleting a post please see: How does deleting work? \$\endgroup\$
    – Mast
    Nov 21 at 6:39
  • 1
    \$\begingroup\$ Again, please do NOT vandalize your posts. \$\endgroup\$
    – Mast
    Nov 21 at 13:23

3 Answers 3

2
\$\begingroup\$

I know nothing about selenium, so I'm going to focus on general streamlining tips. And some of my tips can probably be supplanted by good use of the datetime module that Felix Ogg recommended, but I'll leave those for you to discover.


The expression

if <condition>:
    return True
return False

Can be shortened to

return <condition>

This works in both esta_na_hora() and esta_no_dia_da_semana().


processa_dias_da_semana() is very wasteful. If dia == 'seg', then it will not fulfill any of the other conditions, yet the program checks them anyways. The subsequent checks should use elif instead of if, like so:

if dia == "seg":
    dias_da_semana_int.append(0)
elif dia == "ter":
    dias_da_semana_int.append(1)
elif dia == "qua":
    dias_da_semana_int.append(2)
elif dia == "qui":
    dias_da_semana_int.append(3)
elif dia == "sex":
    dias_da_semana_int.append(4)
elif dia == "sab":
    dias_da_semana_int.append(5)
elif dia == "dom":
    dias_da_semana_int.append(6)

However, you may notice that this is rather repetitive. We can do better still. In Python, an if/elif block where the conditions are all checking the equivalence of a single variable to a series of constants and then assigning a single value as a result can be shortened using a dictionary, like so:

WEEK = {'seg':0, 'ter':1, 'qua':2, 'qui':3, 'sex':4, 'sab':5, 'dom':6}
for dia in dias_da_semana:
    dias_da_semana_int.append(WEEK[dia])

Because the values being assigned are integers from 0 to n, we can also write this using a list:

WEEK = ['seg', 'ter', 'qua', 'qui', 'sex', 'sab', 'dom']
for dia in dias_da_semana:
    dias_da_semana_int.append(WEEK.index(dia))

This is technically less efficient (O(n) time complexity to access a list vs O(1) to access a dict), but for a seven item list the difference is negligible.

Notably, neither of these solutions have the same behavior as your original if dia is not one of the listed possibilities. In your code, the unknown value would be silently ignored, resulting in a final dias_da_semana_int which is shorter than dias_da_semana with no indication as to which values were skipped. My code will instead throw an KeyError or an ValueError depending on which version you chose. Use try/except if you prefer the original behavior, or if you're using a dict you can use WEEK.get(dia, None) to use None (or whatever value you choose) as a default value.

Speaking of which, you make no allowances for different ways the user might enter their text. Your code accepts "seg", but will break on "Seg" or "seg,". It's probably not worth your time to do heavy input normalization if you're going to be the primary user, but you should at least normalize your strings with dia_da_semana_string.lower() to remove capitalization. It's easy to add and eliminates the most common source of input variability.

Finally, your for loop can be replaced by list comprehension. In total, here's a shorter version of processa_dias_da_semana():

WEEK = {'seg':0, 'ter':1, 'qua':2, 'qui':3, 'sex':4, 'sab':5, 'dom':6}
def processa_dias_da_semana(dias_da_semana):
    return [WEEK[dia] for dia in dias_da_semana]

You can also use list comprehensions to efficiently split hora_string

hora, minuto, segundos = [int(ss) for ss in hora_string.split(':')]

Instead of:

ativo = True
while ativo:
    if <condition>:
        ativo = False
        # do stuff

It is cleaner to write this as:

while True:
    if <condition>:
        # do stuff
        break

You can replace i = i+1 with i += 1

\$\endgroup\$
2
  • \$\begingroup\$ Thank you so much!!!! \$\endgroup\$
    – user267072
    Nov 21 at 4:38
  • \$\begingroup\$ @ThiagoSilva If you've found our answers helpful, you should vote them up \$\endgroup\$ Nov 21 at 5:42
3
\$\begingroup\$

It appears that you can detach the browser window to keep it open, see here: https://stackoverflow.com/a/51865955/114266

I recommend you learn the various features of the date and datetime modules (in the Python standard library), which help reduce many lines of your program, e.g. code to index the day of the week using the weekday() function.

https://pythontic.com/datetime/date/weekday

As already mentioned in the comment, but also as general feedback, I would recommend coding in English, i.e. not using Italian variable names, identifiers and so on. I consider Italian a beautiful language, but when you code you will inevitably mix it up with English keywords (while, if) and English module functions ("while navegador.find_element") rendering it hard to read.

Finally, your code would benefit from introducing a function to factor out steps in the long "ativo" section. Yet, considering the code is a one-off to get a job done, not to share with others: If you understand it, and it does the job, that's fine too.

To improve, it depends what you want to achieve. Hardware usage? Performance? I suppose any computer is capable of running this program with a 10-20 threads to "fire" more requests at the site in hopes of winning the appointment slot, but considering that Threaded coding is quite a step up from where you are now, you can also just remove the many "delay" statements you have (they are not needed), and run several instances of the program from your terminal, just open multiple console windows. You do risk of course that you end up taking all the appointment slots they offer, and perhaps being scolded for that :-)

\$\endgroup\$
1
  • \$\begingroup\$ Great ideas! thank you!!! \$\endgroup\$
    – user267072
    Nov 21 at 4:39
2
\$\begingroup\$

To improve this code and make it a bit more robust you should get rid of the time.sleep statements. You have a number of options in Selenium to wait for a certain element to appear like:

from selenium.webdriver.support import expected_conditions

And you can specify a timeout too. See the docs: Selenium Waits And then you can refactor that while loop + the try/except block. In fact you are already using expected_conditions but improperly. That loop is unnecessary but you can keep the try block for exception handling purposes.

I have not tested your code but the xpath selectors look cumbersome. Are you sure there isn't a better way? Try to locate HTML elements by id attribute whenever you can since id attributes are supposed to be unique (but malformed code is still common). This is what you did for the login fields. CSS classes can sometimes be used as a fallback method too. Xpath should be done when the better options are not practicable or too unpredictable.

Likewise, this line of code could probably be simplified:

navegador.find_element(By.XPATH, '//*[@id="dataTableServices"]/tbody/tr[2]/td[4]/a/button').click() #here the system will click on the option Reserve Citizenship

The Xpath selector could be something like this:

//button[@type, 'submit' and text()='Send']

Here you locate the button by its text value. (verify that the control is indeed submit and not button (Javascript) and adjust accordingly).


The instantiation of the Chrome browser does not have to be done repeatedly. It should take place at the beginning, outside your loop (which again is unneeded).


The code is too compact, add some line spacing to improve readability. For example the section where the login takes place should be a detached block.


You put a number of comments but they do not teach me anything because the it's generally obvious what the commented lines do. Instead, focus on commenting the stuff that is less obvious, especially when it's written in another language or implies some logic, for example time calculations.

Had you used English names for your objects and functions, then the code could have been more self-explanatory for an outsider or reviewers like us.


Comments should generally precede the line of code. Inline comments at the end of the line are okay if they are very short (like 1-2 words). Per PEP8 they are separated from the related line by two spaces.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much!!!! \$\endgroup\$
    – user267072
    Nov 21 at 4:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy