4
\$\begingroup\$

This is my first program code ever, and it actually works. My goal is to scrape information from the website and store it in a database. It is a site that has historical data on sporting events and odds.

https://www.oddsportal.com/hockey/sweden/shl-2019-2020/results/

For example, there are 50 matches on this page and the program enters each of them and scrapes this data.

However, I don't think it really looks like the real code. I'm interested in what I can do to improve and optimize it. What do I pay the most attention to, what things do I do wrong?

from selenium import webdriver
from selenium.webdriver.common.action_chains import ActionChains
from selenium.webdriver.common.by import By
from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.support import expected_conditions as EC
from selenium.common.exceptions import NoSuchElementException, TimeoutException
from selenium.webdriver.common.keys import Keys
import sqlite3 as sql
import time

# I disable image display to get up to speed
options = webdriver.ChromeOptions()
prefs = {"profile.managed_default_content_settings.images": 2}
options.add_experimental_option("prefs", prefs)
driver = webdriver.Chrome(options=options)

# Database connection
conn = sql.connect('Hockey_data.db')
c = conn.cursor()

driver.get("https://www.oddsportal.com/hockey/sweden/shl-2019-2020/results/")
driver.maximize_window()

cur_win = driver.current_window_handle

list_of_links = driver.find_elements_by_xpath("//td[2]/a")[0:-2] # because 2 more elements have the same xpath and they are always at the end

for index, link in enumerate(list_of_links):
    link.send_keys(Keys.CONTROL + Keys.RETURN)
    driver.switch_to.window([win for win in driver.window_handles if win !=cur_win][0])  

    # Basic game info
    league = WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.XPATH, "/html/body/div[1]/div/div[2]/div[6]/div[1]/div/div[1]/div[1]/a[4]"))).text

    game = WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.XPATH, "/html/body/div[1]/div/div[2]/div[6]/div[1]/div/div[1]/div[2]/div[1]/h1"))).text
    game_split = game.split('- ')
    home_team, away_team = game_split[0], game_split[1]
    
    time = WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.XPATH, "/html/body/div[1]/div/div[2]/div[6]/div[1]/div/div[1]/div[2]/div[1]/p[1]"))).text
    time_split = time.split(', ')
    day_in_week = time_split[0]
    date = time_split[1]
    day = int(date[0:2])
    month = date[3:7]
    year = int(date[7:11])
    hour = time_split[2]
    
    # Result and goals by periods
    try:
        result = WebDriverWait(driver, 10).until(EC.presence_of_element_located((By.ID, "event-status"))).text
    except NoSuchElementException:
        result = "0000000000000000000000000000000000000000000000000000000000000000000000"
        
    if len(result) == 32:
        home_goals = int(result[13:14])
        home_first = int(result[18:19])
        home_second = int(result[23:24])
        home_third = int(result[28:29])
        home_overtime = 0
        home_penalties = 0
        away_goals = int(result[15:16])
        away_first = int(result[20:21])
        away_second = int(result[25:26])
        away_third = int(result[30:31])
        away_overtime = 0
        away_penalties = 0
    elif len(result) == 33:
        home_goals = int(result[13:15])
        home_first = int(result[19:20])
        home_second = int(result[24:25])
        home_third = int(result[29:30])
        home_overtime = 0
        home_penalties = 0
        away_goals = int(result[16:17])
        away_first = int(result[21:22])
        away_second = int(result[26:27])
        away_third = int(result[31:32])
        away_overtime = 0
    elif len(result) == 40:
        home_goals = int(result[13:14])
        home_first = int(result[21:22])
        home_second = int(result[26:27])
        home_third = int(result[31:32])
        home_overtime = int(result[36:37])
        home_penalties = 0
        away_goals = int(result[15:16])
        away_first = int(result[23:24])
        away_second = int(result[28:29])
        away_third = int(result[33:34])
        away_overtime = int(result[38:39])
        away_penalties = 0
    elif len(result) == 52:
        home_goals = int(result[13:14])
        home_first = int(result[28:29])
        home_second = int(result[33:34])
        home_third = int(result[38:39])
        home_overtime = int(result[43:44])
        home_penalties = int(result[48:49])
        away_goals = int(result[15:16])
        away_first = int(result[30:31])
        away_second = int(result[35:36])
        away_third = int(result[40:41])
        away_overtime = int(result[45:46])
        away_penalties = int(result[50:51])
    else:
        home_goals = 0
        home_first = 0
        home_second = 0
        home_third = 0
        home_overtime = 0
        home_penalties = 0
        away_goals = 0
        away_first = 0
        away_second = 0
        away_third = 0
        away_overtime = 0
        away_penalties = 0

    # 1x2 odds 
    
    pinnacle = "Pinnacle"
    
    try:
        pinnacle_home = driver.find_element_by_xpath("//div[a[contains(text(), 'Pinnacle')]]/following::td[1]")
        hov_pinnacle_home = ActionChains(driver).move_to_element(pinnacle_home)
        hov_pinnacle_home.perform()
        pinnacle_home_closing = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[1]").text
        try:
            pinnacle_home_opening = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[2]").text
        except (NoSuchElementException, TimeoutException):
            pinnacle_home_opening = pinnacle_home_closing 
    except (NoSuchElementException, TimeoutException):
        pinnacle_home = "0000000000"
        pinnacle_home_closing = 0.00
        pinnacle_home_opening = 0.00

    try:
        pinnacle_draw = driver.find_element_by_xpath("//div[a[contains(text(), 'Pinnacle')]]/following::td[2]")
        hov_pinnacle_draw = ActionChains(driver).move_to_element(pinnacle_draw)
        hov_pinnacle_draw.perform()
        pinnacle_draw_closing = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[1]").text
        try:
            pinnacle_draw_opening = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[2]").text
        except (NoSuchElementException, TimeoutException):
            pinnacle_draw_opening = pinnacle_draw_closing 
    except (NoSuchElementException, TimeoutException):
        pinnacle_draw = "0000000000"
        pinnacle_draw_closing = 0.00
        pinnacle_draw_opening = 0.00

    try:
        pinnacle_away = driver.find_element_by_xpath("//div[a[contains(text(), 'Pinnacle')]]/following::td[3]")
        hov_pinnacle_away = ActionChains(driver).move_to_element(pinnacle_away)
        hov_pinnacle_away.perform()
        pinnacle_away_closing = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[1]").text 
        try:
            pinnacle_away_opening = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[2]").text
        except (NoSuchElementException, TimeoutException):
            pinnacle_away_opening = pinnacle_away_closing 
    except (NoSuchElementException, TimeoutException):
        pinnacle_away = "0000000000"
        pinnacle_away_closing = 0.00
        pinnacle_away_opening = 0.00   
        
    average = "Average"

    try:
        average_home = driver.find_element_by_xpath("//td[strong[contains(text(), 'Average')]]/following::td[1]").text
    except NoSuchElementException:
        average_home = 0.00
    try:
        average_draw = driver.find_element_by_xpath("//td[strong[contains(text(), 'Average')]]/following::td[2]").text
    except NoSuchElementException:
        average_draw = 0.00
    try:
        average_away = driver.find_element_by_xpath("//td[strong[contains(text(), 'Average')]]/following::td[3]").text
    except NoSuchElementException:
        average_away = 0.00

    highest = "Highest"

    try:
        highest_home = driver.find_element_by_xpath("//td[strong[contains(text(), 'Highest')]]/following::td[1]").text
    except NoSuchElementException:
        highest_home = 0.00
    try:
        highest_draw = driver.find_element_by_xpath("//td[strong[contains(text(), 'Highest')]]/following::td[2]").text
    except NoSuchElementException:
        highest_draw = 0.00
    try:
        highest_away = driver.find_element_by_xpath("//td[strong[contains(text(), 'Highest')]]/following::td[3]").text
    except NoSuchElementException:
        highest_away = 0.00
    
    # Over-Under tab
    OU_link = WebDriverWait(driver, 5).until(EC.presence_of_element_located((By.XPATH, "/html/body/div[1]/div/div[2]/div[6]/div[1]/div/div[1]/div[2]/div[1]/div[5]/div[1]/ul/li[5]/a/span"))).click()
    
    pinnacle_over_under = "Pinnacle 5.5"
    
    # 5.5 over-under tab
    try:
        OU_55 = WebDriverWait(driver, 5).until(EC.presence_of_element_located((By.XPATH, "//div/div/strong/a[contains(text(), 'Over/Under +5.5')]"))).click()
        
        try:
            pinnacle_over = driver.find_element_by_xpath("//div[a[contains(text(), 'Pinnacle')]]/following::td[2]")
            hov_pinnacle_over = ActionChains(driver).move_to_element(pinnacle_over)
            hov_pinnacle_over.perform()
            pinnacle_over_closing = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[1]").text
            try:
                pinnacle_over_opening = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[2]").text
            except (NoSuchElementException, TimeoutException):
                pinnacle_over_opening = pinnacle_over_closing
        except (NoSuchElementException, TimeoutException):
            pinnacle_over = "0000000000"
            pinnacle_over_opening = 0.00
            pinnacle_over_closing = 0.00

        try:
            pinnacle_under = driver.find_element_by_xpath("//div[a[contains(text(), 'Pinnacle')]]/following::td[3]")
            hov_pinnacle_under = ActionChains(driver).move_to_element(pinnacle_under)
            hov_pinnacle_under.perform()
            pinnacle_under_closing = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[1]").text
            try:
                pinnacle_under_opening = driver.find_element_by_xpath("//*[@id='tooltiptext']/strong[2]").text
            except (NoSuchElementException, TimeoutException):
                pinnacle_under_opening = pinnacle_over_closing
        except (NoSuchElementException, TimeoutException):
            pinnacle_under = "0000000000"
            pinnacle_under_opening = 0.00
            pinnacle_under_closing = 0.00
        
    except (NoSuchElementException, TimeoutException):
        OU_55 = "0000000000"
        pinnacle_under_opening = 0.00
        pinnacle_under_closing = 0.00
        
    average_over_under = "Average 5.5"

    try:
        average_over = driver.find_element_by_xpath("//td[strong[contains(text(), 'Average')]]/following::td[2]").text
    except NoSuchElementException:
        average_over = 0.00
    try:
        average_under = driver.find_element_by_xpath("//td[strong[contains(text(), 'Average')]]/following::td[3]").text
    except NoSuchElementException:
        average_under = 0.00

    highest_over_under = "Highest 5.5"

    try:
        highest_over = driver.find_element_by_xpath("//td[strong[contains(text(), 'Highest')]]/following::td[2]").text
    except NoSuchElementException:
        highest_over = 0.00
    try:
        highest_under = driver.find_element_by_xpath("//td[strong[contains(text(), 'Highest')]]/following::td[3]").text
    except NoSuchElementException:
        highest_under = 0.00
        
    c.execute('insert into Model values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)', 
              (league, home_team, away_team, day_in_week, day, month, year, hour,
               home_goals, away_goals, home_first, away_first, home_second,away_second, home_third, away_third,home_overtime, away_overtime, home_penalties, away_penalties,
               pinnacle, pinnacle_home_closing, pinnacle_draw_closing, pinnacle_away_closing, pinnacle_home_opening, pinnacle_draw_opening, pinnacle_away_opening,
               average, average_home, average_draw, average_away, highest, highest_home, highest_draw, highest_away,
               pinnacle_over_under, pinnacle_over_closing, pinnacle_under_closing, pinnacle_over_opening, pinnacle_under_opening,
               average_over_under, average_over, average_under, highest_over_under, highest_over, highest_under,)) 
    
    conn.commit()  
    
    print("Number: ", index) 

    driver.close()
    driver.switch_to.window(cur_win)
\$\endgroup\$
3
\$\begingroup\$

Repetition 1

You have several XPATHs which are largely the same. Especially when they share a common prefix, like these

"/html/body/div[1]/div/div[2]/div[6]/div[1]/div/div[1]/div[1]/a[4]"

I would declare a variable

xpath_base = "/html/body/div[1]/div/div[2]/div[6]/div[1]/div/div[1]"

and then reuse that for every xpath that has the same base. This makes the code more readable and easier to modify in case the base changes.

Your first xpath then becomes xpath_base + "/div[1]/a[4]" , your second becomes xpath_base + "/div[2]/div[1]/h1" and so on.

Reptition 2 / Mapping

I would prefer to use some more compact structure for the goals reading. Not sure if you find it more readable, but it is shorter.

You could put your indexes in a combination of dict and arrays like this.

In this case I'm only using the indexes for home and away goals, first, second, third since they are used with indexes in all 4 cases.

result_mapping = {
    32: [13, 14, 18, 19, 23, 24, 28, 29, 15, 16, 20, 21, 25, 26, 30, 31],
    33: [13, 15, 19, 20, 24, 25, 29, 39, 16, 17, 21, 22, 26, 27, 31, 32],
    40: [...],
    52: [...]
}

With this mapping you can then do

if len(result) in result_mapping.keys():
    indexes = result_mapping[len(result)]
    home_goals = int(result[indexes[0] : indexes[1]])
    home_first = int(result[indexes[2] : indexes[3]])
    home_second = int(result[indexes[4] : indexes[5]])

and so on. There is more room for making this code shorter, like making a function for int(result[]) since that is repeated on each line.

Also, the indexes are always n and n+1 except for home_goals = int(result[13:15]) (is that an error in the indexing?). If the 13:15 is an error and supposed to be 13:14 then you can remove all the end indexes in the arrays above, so

32: [13, 14, 18, 19, 23, 24, 28, 29, 15, 16, 20, 21, 25, 26, 30, 31]

becomes

32: [13, 18, 23, 28, 15, 20, 25, 30]

and you just add +1 instead of the end index, in each case.

Repetition 3 / Aliasing

Things that you repeat a lot, I would prefer to make a shorter alias for. You could define something like xpath = driver.find_element_by_xpath early in the code and then use the new name to call that function.

Repetition 4 / Default cases

In the long if-elif-else piece, you have your default case last after the else.

But some of the cases here are shared with several of the cases in the if-else. For that reason, it would make more sense to define your defaults first, before the if-else. Since they are all zero, you can also make them a bit shorter like this.

home_goals = home_first = home_second = home_third = home_overtime = home_penalties = 0

Now by doing this before the if-else, you don't need to mention these variables again in the cases that they should still be 0 such as home_penalties in 3 out of 4 cases.

Final comment

I don't think it really looks like the real code

I think one reason it doesn't look like "real code" is that it has so many try - except and special cases and so on. But that is hard to avoid when writing a scraper for a particular website. You need to write custom code for the structure of that website, and you can't do a lot of the refactorings / shortcuts and reuse of things that you normally can when writing a normal program. So I think most of the code looks fine for this use case.

One more addition after posting

You have plenty of cases following the same pattern, like this

try:
    average_under = driver.find_element_by_xpath("//td[strong[contains(text(), 'Average')]]/following::td[3]").text
except NoSuchElementException:
        average_under = 0.00

You can refactor this and make it a lot less repetitive by creating a function.

average_under = try_or_default("//td[strong[contains(text(), 'Average')]]/following::td[3]", 0.00)

and your function will be

def try_or_default(xpath, default_value):
    try:
        return driver.find_element_by_xpath(xpath).text
    except NoSuchElementException:
        return default_value 

(I haven't tried running it so there may be some syntax error).

You can make it further shorter by setting a default for the default_value

def try_or_default(xpath, default_value=0.00):

Which allows you to leave out the 0.00 when calling the function in most cases.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ First, thank you so much for your effort and response. Repetition 1 / Declaring variables with the same base - Clear. Reptition 2 / Mapping - I'll try to do this, it looks interesting. home_goals = int(result[13:15]) This is not a mistake, but refers to rare situations where the home team scores a double-digit number of goals (if the guest scores a double-digit number, then I'm in trouble :)) Repetition 3 / Aliasing - Sounds very reasonable, I will try to respect this in the future. Repetition 4 / Default cases - Clear. Thanks again. \$\endgroup\$ – Dejan Samardzija Sep 13 at 10:51
  • \$\begingroup\$ added one more suggestion, please see the edited answer \$\endgroup\$ – user985366 Sep 13 at 11:11
  • \$\begingroup\$ Great recommendation, works without problem! Thank you, I learned a lot \$\endgroup\$ – Dejan Samardzija Sep 13 at 15:44

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.