2
\$\begingroup\$

It's been nearly a year I've been working with C++ and now I am diving in Python and its libraries and frameworks. I am currently creating a website for my portfolio that gathers news articles from a wide range of news websites. I am using Django framework for web development and the Beautiful Soup library for web scraping.

This is currently my Django views.py file, where all the web scraping happens:

from django.shortcuts import render

import urllib.request
from bs4 import BeautifulSoup
import re

# More websites will be added below.
URLS = ["https://www.theguardian.com/uk/rss", "http://rss.cnn.com/rss/edition.rss"];

class Brick:
    """
    Class to represent each article in a news website. 1 brick = 1 news article.
    """
    def __init__(self, title="Untitled", link="#", description="No description",
                 img_url="#", creator="No creator", date="No Date"):
        self.title = title;
        self.link = link;
        self.description = description;
        self.img_url = img_url;
        self.creator = creator;
        self.date = date;

def strip_html(raw_html):
    """
    Strips any HTML tags within a string.
    (I know that this is not safe, but I would like to practice regular expressions).
    """
    pattern = re.compile(r"<.*?>");
    return pattern.sub("", raw_html);

def shorten(cleaned_html):
    """
    If the description of an article is too long, let's cut it
    to only 50 characters.
    """
    new_string = "";

    index = 0;
    for c in cleaned_html:
        if index < 100:
            new_string += cleaned_html[index];

        index += 1;

    new_string += "...";
    return new_string;

# Create your views here.
def news_list(request):
    """
    View that will redirect to the home template that shows the full list
    of articles.
    """

    unformatted_xmls = [];

    formatted_xmls = [];

    group_of_items = [];

    for URL in URLS:
        with urllib.request.urlopen(URL) as file_object:
            unformatted_xmls.append(file_object.read());

    for unf_xml in unformatted_xmls:
        formatted_xmls.append(BeautifulSoup(unf_xml, "xml"));


    for form_xml in formatted_xmls:
        group_of_items.append(form_xml.find_all("item"));

    bricks = [];

    for item_group in group_of_items:
        for item in item_group:
            b = Brick();
            b.title = item.find("title").get_text();
            b.link = item.find("link").get_text();
            b.description = shorten(strip_html(item.find("description").get_text())) if item.find("description") is not None else "No description.";
            b.img_url = item.find("media:content").get("url") if item.find("media:content") is not None else "No media.";
            b.creator = item.find("dc:creator").get_text() if item.find("dc:creator") is not None else "No creator.";
            b.date = item.find("dc:date").get_text() if item.find("dc:date") is not None else "No date.";
            bricks.append(b);

    return render(request, "news/list.html", {"bricks": bricks});

As I am new to web scraping and web development in general, would you mind giving me your tips and opinions about this code? Would you recommend any good practices? Are these libraries and frameworks strongly recommended for this purpose?

\$\endgroup\$
1
\$\begingroup\$

It seems wasteful to re-crawl the specified websites every time your web page gets hit, particularly if the web pages you're crawling have nothing to do with the request sent to your webpage. This seems like a job that should be run as a cron job (say, every 5 or 30 minutes), stored in the database, then simply fetched from the database when your webpage gets hit.

A great framework for more advanced crawling is scrapy. It very likely might be overkill for this application, but if you're pondering more advanced scraping, it's a good utility for that purpose.


Now to actually review your code.

Your shorten function could be radically simplified using slices:

def shorten(cleaned_html, limit=50):
    """
    If the description of an article is too long, let's cut it
    to only 50 characters.
    """
    return cleaned_html[:limit] + ('...' if len(cleaned_html) > limit else '')

Also, in strip_html, you compile your regex every time, but only use it once. In Python's re module, there are shortcut functions when you're only using a regex once. You could simply do return re.sub(r"<.*?>", "", raw_html)

When doing your crawling, you create several temporary lists, each of which is a simple function that transforms the elements of the previous list, which you do using a list. In Python, there's a very efficient way (both in memory and compute time) to represent this using generator comprehensions. You also have a robust constructor for Brick; instead of creating a generic Brick and then modifying it, just make the exact Brick you want in the first place:

def extract_brick(item):
    return Brick(
        title=item.find("title").get_text(),
        link=item.find("link").get_text(),
        description=shorten(strip_html(item.find("description").get_text())) if item.find("description") is not None else "No description.",
        img_url=item.find("media:content").get("url") if item.find("media:content") is not None else "No media.",
        creator=item.find("dc:creator").get_text() if item.find("dc:creator") is not None else "No creator.",
        date=item.find("dc:date").get_text() if item.find("dc:date") is not None else "No date."
    )

def news_list(request):
    unformatted_xmls = (urllib.request.urlopen(URL) for URL in URLS)
    formatted_xmls = (BeautifulSoup(unf_xml, "xml") for unf_xml in unformatted_xmls)
    group_of_items = (form_xml.find_all("item") for form_xml in formatted_xmls)
    bricks = [extract_brick for item_group in group_of_items for item in item_group]

    return render(request, "new/list.html", {"bricks": bricks})

And lastly, you don't need semi-colons at the end of every line. It's a hard habit to break when transitioning from languages where whitespace doesn't really matter, such as C++, C, or Java, but in Python they're mostly just clutter.

\$\endgroup\$

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.