3
\$\begingroup\$

I wrote this code some time ago as part of the web-scraping learning. Every now and then I find mistakes in it, as well as I have doubts. Feedback please, is this code compliant with the common best practices? Any comments will be useful.

# crawler_her_sel.py
# -*- coding: utf-8 -*-

import time

from selenium.webdriver import Firefox
from selenium.webdriver.firefox.options import Options
from selenium.webdriver.common.by import By
from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.support import expected_conditions as EC
from bs4 import BeautifulSoup
import pandas as pd

# Variable with the URL of the website.
my_url = "https://www.flashscore.com/"

# Preparing of the browser for the work.
options = Options()
options.add_argument("--headless")
driver = Firefox(options=options)
driver.get(my_url)

# Prepare the blank dictionary to fill in for pandas.
dictionary_of_matches = {}

# Preparation of lists with scraped data.
list_of_countries = []
list_of_leagues = []
list_of_home_teams = []
list_of_scores_for_home = []
list_of_scores_for_away = []
list_of_away_teams = []

# Wait for page to fully render
try:
    element = WebDriverWait(driver, 25).until(
        EC.presence_of_element_located((By.CLASS_NAME, "adsclick")))
finally:
    # Loads the website code as the BeautifulSoup object.
    pageSource = driver.page_source
    bsObj = BeautifulSoup(pageSource, "lxml")

    # Determining the number of the football matches with the help of 
    # the BeautifulSoup.
    games_1 = bsObj.find_all("div", {"class": 
        "event__participant event__participant--home"})
    games_2 = bsObj.find_all("div", {"class": 
        "event__participant event__participant--home fontBold"})
    games_3 = bsObj.find_all("div", {"class": 
        "event__participant event__participant--away"})
    games_4 = bsObj.find_all("div", {"class": 
        "event__participant event__participant--away fontBold"})

    # Determining the number of the countries for the given football 
    # matches.
    countries = driver.find_elements(By.CLASS_NAME, "event__title--type")

    # Determination of the number that determines the number of 
    # the loop iterations.
    sum_to_iterate = len(countries) + len(games_1) + len(games_2) 
    + len(games_3) + len(games_4)
    
    for ind in range(1, (sum_to_iterate+1)):
        # Scraping of the country names.
        try:
            country = driver.find_element(By.XPATH, 
                '//div[@class="sportName soccer"]/div['+str(ind)+
                ']/div[2]/div/span[1]').text
            list_of_countries.append(country)
        except:
            country = ""
            list_of_countries.append(country)

        # Scraping of the league names.
        try:
            league = driver.find_element(By.XPATH, 
                '//div[@class="sportName soccer"]/div['+str(ind)+
                ']/div[2]/div/span[2]').text
            list_of_leagues.append(league)
        except:
            league = ""
            list_of_leagues.append(league)

        # Scraping of the home team names.
        try:
            home_team = driver.find_element(By.XPATH,
                '//div[@class="sportName soccer"]/div['+str(ind)+
                ']/div[3]').text
            list_of_home_teams.append(home_team)
        except:
            home_team = ""
            list_of_home_teams.append(home_team)

        # Scraping of the home team scores.
        try:
            score_for_home_team = driver.find_element(By.XPATH, 
                '//div[@class="sportName soccer"]/div['+str(ind)+
                ']/div[5]').text
            list_of_scores_for_home.append(score_for_home_team)
        except:
            score_for_home_team = ""
            list_of_scores_for_home.append(score_for_home_team)

        # Scraping of the away team scores.
        try: 
            score_for_away_team = driver.find_element(By.XPATH, 
                '//div[@class="sportName soccer"]/div['+str(ind)+
                ']/div[6]').text
            list_of_scores_for_away.append(score_for_away_team)
        except:
            score_for_away_team = ""
            list_of_scores_for_away.append(score_for_away_team)

        # Scraping of the away team names.
        try:
            away_team = driver.find_element(By.XPATH, 
                '//div[@class="sportName soccer"]/div['+str(ind)+
                ']/div[4]').text
            list_of_away_teams.append(away_team)
        except:
            away_team = ""
            list_of_away_teams.append(away_team)

    # Add lists with the scraped data to the dictionary in the correct 
    # order.
    dictionary_of_matches["Countries"] = list_of_countries
    dictionary_of_matches["Leagues"] = list_of_leagues
    dictionary_of_matches["Home_teams"] = list_of_home_teams
    dictionary_of_matches["Scores_for_home_teams"] = list_of_scores_for_home
    dictionary_of_matches["Scores_for_away_teams"] = list_of_scores_for_away
    dictionary_of_matches["Away_teams"] = list_of_away_teams

    # Creating of the frame for the data with the help of the pandas 
    # package.
    df_res = pd.DataFrame(dictionary_of_matches)

    # Saving of the properly formatted data to the csv file. The date 
    # and the time of the scraping are hidden in the file name.
    name_of_file = lambda: "flashscore{}.csv".format(time.strftime(
        "%Y%m%d-%H.%M.%S"))
    df_res.to_csv(name_of_file(), encoding="utf-8")

    driver.quit()
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Are you using Python2? If not the utf-8 declaration on top of your page isn't needed.

Obviously there is a lot of duplication as you repeat the same code to do the same thing. All you need is to write an ad hoc function that accepts an xpath expression and returns a string (or a list). This would reduce your code base instantly.

You already have an xpath expression at line 56 to fetch the list of countries:

countries = driver.find_elements(By.CLASS_NAME, "event__title--type")

Then you should already have a list that can be iterated. Thus, further scraping of country names in a loop probably is not necessary. Same thing for the other items. Do you really need that loop? You are just doing xpath selection. And it would make more sense to fetch the different types of items separately and not within the same loop, since their respective lengths can vary.

Note that you are using a Selenium function here, whereas BS exposes a similar function (find_all). Indeed you already use BS find_all before. Then I would suggest sticking to BS. Using both at the same time can be a source of confusion.

This code is wasteful because you could fetch several tags at the same time:

games_1 = bsObj.find_all("div", {"class": 
    "event__participant event__participant--home"})
games_2 = bsObj.find_all("div", {"class": 
    "event__participant event__participant--home fontBold"})
games_3 = bsObj.find_all("div", {"class": 
    "event__participant event__participant--away"})
games_4 = bsObj.find_all("div", {"class": 
    "event__participant event__participant--away fontBold"})

This should accomplish the same thing:

games = bsObj.find_all("div", {"class":[
    "event__participant event__participant--home",
    "event__participant event__participant--home fontBold",
    "event__participant event__participant--away",
    "event__participant event__participant--away fontBold"
]}

This is a list, but you could even use a regular expression for more flexibility.

A more Pythonic loop would not use range but look like:

countries = driver.find_elements(By.CLASS_NAME, "event__title--type")
for country in countries:
    ...

As regards naming of variables: the list_ prefix isn't meaningful eg: prefix list_of_countries. countries is enough and to the point. A good IDE should tell you the variable data type anyway.

The finally statement at line 38 is misplaced. The code in a finally block is always executed, even after an exception occurs. This is not what you want. Instead, add an except block and stop your application if page loading fails. Because if it does, then the rest of the code cannot complete and it would be pointless to proceed.

The way you use the except clause is problematic because you are ignoring any errors that may occur, and clearly a web scraping application can fail in so many ways. That makes debugging difficult too. I really recommend that you stop in case of error, because your application is bound to behave unpredictably, or return garbage in this case. Some types of exceptions can be anticipated and handled gracefully but you're not doing any of that presently. Don't mask the exception, show the details so you can figure out what went wrong.

To build the file name, an F-string would be more appropriate, provided that you run Python >= 3.6. The lambda expression is not needed.

Using Pandas to generate a CSV file is overkill because you are not doing any sophisticated data processing that would mandate using that library. The built-in csv module should be sufficient for your needs.

\$\endgroup\$
2

Your Answer

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

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