Beginner Python OOP web scraper

Question

I started learn OOP in Python. I have OOP basics in Java and apparently I have a problem with thinking in OOP in Python (and using the best functionalities and syntax). I have doubts about my OOP design in web scraping for a local book review page. So the basic idea is scraping information about book with the user reviews for that books.

The structure of the Python package:

/bookscraper
    __init__.py
    book.py
    comment.py
    parsers.py

parsers.py:

from urllib.request import urlopen
from bs4 import BeautifulSoup
from bookscraper.book import Book
from bookscraper.comment import Comment


class Parser:
    def __init__(self, url, name, search_url):
        self.url = url
        self.name = name
        self.search_url = search_url


class DatabazeKnih(Parser):
    PAGE_URL = 'http://www.databazeknih.cz/'
    NAME = 'Databáze knih'
    SEARCH_URL = 'search?q={}&hledat=&stranka=search'

    def __init__(self):
        super().__init__(DatabazeKnih.PAGE_URL, DatabazeKnih.NAME, DatabazeKnih.SEARCH_URL)

    def search(self, name):
        self.search_html = urlopen(DatabazeKnih.PAGE_URL + DatabazeKnih.SEARCH_URL.format(name))
        self.book_link = DatabazeKnih.PAGE_URL + self.get_book_link() + '?show=alldesc'
        return self.get_book_info()

    def get_book_page(self):
        return urlopen(self.book_link)

    def get_book_link(self):
        soup = BeautifulSoup(self.search_html, 'html.parser')
        return soup.find(type='book').attrs['href']

    @staticmethod
    def parse_comment(comment):
        user = comment.find('a').string
        text = comment.find('p', {'class': 'odtopm new2 justify'}).text
        date = comment.find('span', {'class': 'pozn_lightest odleft_pet'}).string
        return user, text, date

    @staticmethod
    def get_comments(soup):
        comments_objects = []
        comments = soup.find_all('div', {'class': 'komholdu'})
        for comment in comments:
            user, text, date = DatabazeKnih.parse_comment(comment)
            comments_objects.append(Comment(user, text, date))
        return comments_objects

    @staticmethod
    def get_authors(soup):
        return [author.string for author in soup.find(itemprop='author').find_all('a')]

    def get_book_info(self):
        soup = BeautifulSoup(self.get_book_page(), 'html.parser')
        title = soup.find(itemprop='name').string
        rating = soup.findAll('a', {'class': 'bpoints'})[0].string
        text = soup.find(itemprop='description').string
        authors = DatabazeKnih.get_authors(soup)
        year = soup.find(itemprop='datePublished').string
        comments = DatabazeKnih.get_comments(soup)
        return Book(title, rating, text, authors, year, comments)

Then there are the Book and Comment classes. I will add some functionality to these classes. Now I prepared only constructors:

class Book:
    def __init__(self, title, rating, text, authors, year, comments):
        self.title = title
        self.rating = rating
        self.text = text
        self.authors = authors      # this is a list
        self.year = year
        self.comments = comments    # this is a list

class Comment:
    def __init__(self, user, text, date):
        self.user = user
        self.text = text
        self.date = date

Then I can't get info about book via creating parser object:

from bookscraper.parsers import DatabazeKnih

parser = DatabazeKnih()
book = parser.search('ydris')  # Name of the book

My code is working, but is it a good OOP design? I have some doubts about the design of methods in the DatabazeKnih class. In search method I set two attributes I found it is not a good practice, but how should I rewrite? Should I create setters for this two attributes?

Answer 1

Use list comprehensions instead of creating empty array and appending to it in a for loop. It's both faster and more readable.

@staticmethod
def get_comments(soup):
    return [
        Comment(*DatabazeKnih.parse_comment(comment)
        for comment in soup.find_all('div', {'class': 'komholdu'})
    ]

But If would argue that you don't need classes here at all.

from bs4 import BeautifulSoup
from collections import namedtuple
import requests as r


Book = namedtuple('Book', 'title rating text authors year comments')
Comment = namedtuple('Comment', 'user text date')
PAGE_URL = 'http://www.databazeknih.cz/'


def search(name):
    page = r.get(PAGE_URL + 'search', params=dict(q=name, stranka='search')).text
    link = BeautifulSoup(page, 'html.parser').find(type='book').attrs['href']
    book_page = r.get(PAGE_URL + link, params=dict(show='alldesc')).text
    return book_info(book_page)


def book_info(book_page):
    soup = BeautifulSoup(book_page, 'html.parser')
    title = soup.find(itemprop='name').string
    rating = soup.findAll('a', {'class': 'bpoints'})[0].string
    text = soup.find(itemprop='description').string
    authors = [elem.string for elem in soup.find(itemprop='author').find_all('a')]
    year = soup.find(itemprop='datePublished').string
    comments = [
        Comment(*parse_comment(elem))
        for elem in soup.find_all('div', {'class': 'komholdu'})
    ]
    return Book(title, rating, text, authors, year, comments)


def parse_comment(comment):
    user = comment.find('a').string
    text = comment.find('p', {'class': 'odtopm new2 justify'}).text
    date = comment.find('span', {'class': 'pozn_lightest odleft_pet'}).string
    return user, text, date

My suggestions are.

Use requests library, It's more pythonic than urllib.

Use namedtuples instead of classes where possible. Easier to read and to write. If you need to add functionality you can inherit from namedtuple.

Don't write one liner get_this get_that functions. It's useful when you're hiding some implementation, but hiding one or two lines - it's useless and it's even a bit less readable (when you would need this implementation).

What you're doing with function name get_authors you can do just as easy with giving a variable self-explanatory name like authors = .... Everybody would understand that next one or two lines (if it's a comprehension) is getting authors in some way and they can read this lines if they want details.

Also get_book_info means suggesting it's loading or getting something from somewhere. In java it just means that this is some property, but in python we have @property decorator and don't need getters and setters so get is used when you're loading something. Instead it's more accurate to think of it as pure function that takes html and outputs Book. You can call it make_book_info, def book_info_from(html): or just book_info.

Answer 2

I can tell that this is a very well written scraper, I don't feel like there are any problems with the code. There is some PEP8 stuff that you can be done but I'm not going to mention it because it's minor and doesn't, in fact, improve the code. The code is soundly organized, classes are very nicely organized not too many files All I would suggest are Comments. Your code is already self-explanatory but comments are a really useful way for someone else to continue your work or to understand it.

Note: I'm probably not qualified enough to talk to you about this, however, I've been using Python and BeautifulSoup for quite some time now and that's why I'm here.