11
\$\begingroup\$

I have made the following program to gather data on random books from Goodreads, via their random books feature.

import requests
import re

URL = "https://www.goodreads.com/book/random"

while True:
    html_text = requests.get(URL).text

    #   Rating Count
    bg_rating_count     = html_text.find("<span class=\"value-title\" title=\"") + 33
    end_rating_count    = html_text.find("\" itemprop=\"ratingCount\">", bg_rating_count)
    rating_count        = int(html_text[bg_rating_count : end_rating_count].replace(',', ''))

    if rating_count >= 30:
        if "<span itemprop=\"numberOfPages\">" in html_text:

            #   Title
            bg_title    = html_text.find("<meta property=\"og:title\" content=\"") + 35
            end_title   = html_text.find("\"/>", bg_title)
            title = html_text[bg_title : end_title].replace("&amp;", '&')
            title = re.sub(r'\((.*)\)','', title)

            #   Pages
            bg_pages    = html_text.find("<span itemprop=\"numberOfPages\">") + 31
            end_pages   = html_text.find(" page", bg_pages)
            pages       = int(html_text[bg_pages : end_pages])

            #   Rating
            bg_rating   = html_text.find("<span class=\"average\" itemprop=\"ratingValue\">") + 45
            end_rating  = html_text.find("</", bg_rating)
            rating      = float(html_text[bg_rating : end_rating])

            if pages != 0:
                print(title, pages, rating, sep='\t')

This is my first time working with anything of this sort, and I am still learning Python as I go. Here's a breakdown of what the code does (or, is intended to do):

  1. This program gets the HTML code from a random book via the URL.

  2. Finds the book's rating count by searching for a specific HTML tag. Removes any commas in the rating number before making sure it's an integer.

  3. If the rating count is >= 30, it accepts it and continues to gather data. Otherwise it moves on and tries another random book.

  4. If it passes the rating count test, it checks whether it has a listed number of pages. If it does, it continues to gather data. Otherwise it moves on and tries another random book.

  5. If it passes the page count test, it then gathers the title. If the title has an ampersand HTML code &amp; it replaces it with the actual ampersand character &. As well, if the title contains text identifying a series, it removes the series text. Here's an example of what I'm talking about, where it says (The Hunger Games #1). I am new to regex, let me know if I am doing anything horrible.

  6. It then actually gathers the page count along with making sure it's an integer, and the book's rating with making sure it's a float.

  7. If the page count is not 0 (which is sometimes the case for ill-detailed books), it then prints out the relevant data in a tab-separated format that I can copy-paste for later.

I understand that this program goes on forever, and I intended it like that. This program is meant to be manually used, and I halt it when I am done.

Is multi-threading a good idea? How can I improve this code, both in terms of performance, readability, or code logic?

\$\endgroup\$
21
\$\begingroup\$

After I've gone through your code, I decided to rewrite it because using regex to parse html isn't a good idea at all.

When you're parsing html, is recommended to use BeautifulSoup, so I'll rewrite your code using it.


First, I'll start from the imports:

from bs4 import BeautifulSoup as bs
import requests

Nothing too fancy so far, we're importing the modules we need in order to have our work done.


You had in your code a magic number (30), which we can define at the top of our program, just below the imports:

MIN_RATING_COUNT = 30 

You had other numbers in your code which didn't make sense to me, so I removed them. Just let me know what their purpose was (if any) if there's any difference between my proposed solution and yours.


Moving on, we can now build a function which returns a bs object on which we can later work.

def get_html_source():
    """Docstring here."""
    html_source = requests.get(URL).text
    return bs(html_source, 'html.parser')

Now, let's build another four functions which will get us the rating count, title, pages and rating of a random book.

def get_book_rating_count(soup):
    """Docstring here."""
    return soup.find('span', attrs={'class', 'value-title'}).get_text()


def get_book_title(soup):
    """Docstring here."""
    return soup.find('h1', attrs={'class': 'bookTitle'}).get_text()


def get_book_pages(soup):
    """Docstring here."""
    return soup.find('span', attrs={'itemprop': 'numberOfPages'}).get_text()


def get_book_rating(soup):
    """Docstring here."""
    return soup.find('span', attrs={'itemprop': 'ratingValue'}).get_text()

As I was testing this out, I've noticed that book_rating_count might have different value formats so let's build another function to treat each case:

def to_float(rating_count):
    """Docstring here."""
    rating = rating_count.split()[0]
    return float(rating.replace(',', '.'))

Regarding the title, we can build another function to nicely format it: remove the newlines in it and replace &amp; with &:

def format_title(book_title):
    """Docstring here."""
    return ' '.join(book_title.split()).replace('&amp;', '&')

Last, but not least, let's build our main function:

def main():
    """Docstring here."""
    while True:
        soup = get_html_source()
        book_rating_count = get_book_rating_count(soup)

        if to_float(book_rating_count) > MIN_RATING_COUNT:
            book_pages = get_book_pages(soup)
            book_title = format_title(get_book_title(soup))
            book_rating = get_book_rating(soup)

            print('Title: {}\n'
                  'Pages: {}\n'
                  'Rating: {}\n\n'.format(book_title, book_pages, book_rating))

If you're also going to terminate the program using CTRL + C, I suggest you put your main function into a try/except block.

try:
    main()
except KeyboardInterrupt:
    print("You've decided to close the program")

The final code:

from bs4 import BeautifulSoup as bs
import requests

URL = "https://www.goodreads.com/book/random"
MIN_RATING_COUNT = 30


def to_float(rating_count):
    """Docstring here."""
    rating = rating_count.split()[0]
    if ',' in rating:
        return float(rating.replace(',', '.'))
    return float(rating)


def format_title(book_title):
    """Docstring here."""
    return ' '.join(book_title.split()).replace('&amp;', '&')


def get_html_source():
    """Docstring here."""
    html_source = requests.get(URL).text
    return bs(html_source, 'html.parser')


def get_book_rating_count(soup):
    """Docstring here."""
    return soup.find('span', attrs={'class', 'value-title'}).get_text()


def get_book_title(soup):
    """Docstring here."""
    return soup.find('h1', attrs={'class': 'bookTitle'}).get_text()


def get_book_pages(soup):
    """Docstring here."""
    return soup.find('span', attrs={'itemprop': 'numberOfPages'}).get_text()


def get_book_rating(soup):
    """Docstring here."""
    return soup.find('span', attrs={'itemprop': 'ratingValue'}).get_text()


def main():
    """Docstring here."""
    while True:
        soup = get_html_source()
        book_rating_count = get_book_rating_count(soup)

        if to_float(book_rating_count) > MIN_RATING_COUNT:
            try:
                book_pages = get_book_pages(soup)
            except AttributeError:
                book_pages = 'No pages available'
            book_title = format_title(get_book_title(soup))
            book_rating = get_book_rating(soup)

            print('Title: {}\n'
                  'Pages: {}\n'
                  'Rating: {}\n\n'.format(book_title, book_pages, book_rating))

if __name__ == '__main__':
    try:
        main()
    except KeyboardInterrupt:
        print("You've decided to close the program")

If you let the above run for several minutes, you'll eventually get an AttributeError for the simple fact that some books don't have the number of pages, so I also added that into a try/except block.

Other changes that I've done:


Recommendations regarding your code-style:

  • in comments, there should be only one space after #;
  • there should be only one space before and after = operator;
  • instead of randomly using comments inside your code, just add docstrings to your functions (see PEP257 for more info);
  • instead of name = "He called me \"Bla\" again!" you can do name = 'He called me "Bla" again!' and vice-versa.

You can also read more about Python's style-guide, here.


Performance:

If you'll have many <30 books one ofter another, you don't have what to do but wait. As for your question regarding multithreading, threads only actually speed up I/O operations - because of the GIL.

CPU-bound operations (like the parsing/searching BeautifulSoup is doing) can't actually be done in parallel via threads, because only one thread can do CPU-based operations at a time. So you still may not see the speed up you were hoping for with this approach.

When you need to speed up CPU-bound operations in Python, you need to use multiple processes instead of threads. Luckily, you can easily do this using the multiprocessing module.

\$\endgroup\$
  • \$\begingroup\$ Thanks, this code is far nicer. I made some changes to the code. Inside get_book_pages, I added code to remove the text pages or page after the number (I'm copy-pasting, I only want the #). I added regex to remove the series text (Series txt here) from the title string. I changed the code so it goes on to another book if it can't find page #. I changed the way it prints out the final data, so it matches the way I was printing it out before. Changed > to >=, and to_float to to_int Here's the final code.. \$\endgroup\$ – esote Apr 29 '17 at 18:06
  • \$\begingroup\$ I don't think importing BeautifulSoup as bs makes much sense if you use it only once. I've only seen that type of shorthand used for things like numpy, where every other function is part of the numpy package. \$\endgroup\$ – Blender Apr 30 '17 at 5:50
  • 1
    \$\begingroup\$ @Blender you're right. It became more of a reflex as I'm using it a lot more when I'm writing a script like this ;) \$\endgroup\$ – Grajdeanu Alex. Apr 30 '17 at 8:01
  • 1
    \$\begingroup\$ In your to_float function you don't have to check if the string includes a ','. str.replace will just not replace anything in that case. \$\endgroup\$ – Graipher Apr 30 '17 at 15:14

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.