4
\$\begingroup\$

I'm trying to improve my pretty basic coding skills, and I've written this program to scrape a jobs board (I'd ultimately add several) and put the data in a db file with SQLITE3. The code works, but I can't help feeling that there are some simple things I might have done more gracefully.

One error I know about is that I don't have a primary key for my DB. I'm going to add this! In the meantime, I've just been querying the DB with something like SELECT DISTINCT Title, Company FROM jobs WHERE Date BETWEEN datetime('now', '-3 days') AND datetime('now', 'localtime'); which works nicely.

Thanks in advance for any feedback!

import csv
import datetime
import urllib.request
import sqlite3
from sqlite3 import Error
from bs4 import BeautifulSoup
from dateutil import parser

"""
Scrapes STA jobs board, adds new jobs into a database
"""


def update_db():
    try:
        conn = sqlite3.connect('jobs.db')
        c = conn.cursor()

        # call the scraping functions
        soup = scrape_secret()
        jobs = clean_jobs(soup)
        result = organise(jobs)
        final = data_cleanser(result)
        export(final)

        # after exporting to csv (just in case) we delete the title row and convert nested lists to tuples
        del final[0]
        new_result = [tuple(l) for l in final]
        # only necessary once
        # c.execute('''CREATE TABLE jobs (Title, Company, Location, Type, Date Posted)''')

        c.executemany("INSERT INTO jobs VALUES (?,?,?,?,?)", new_result)
        conn.commit()
        conn.close()

    except Error as e:
        print(e)


# function to remove multiple occurrences of one term ('new')
def remove_value_from_list(the_list, val):
    return [value for value in the_list if value != val]


def length_enforcer(the_list, length):
    return [value for value in the_list if len(value) == length]


# hit the website and scrape the first page
def scrape_secret():
    url = "https://jobs.secrettelaviv.com/"
    req = urllib.request.Request(url, headers={'User-Agent': 'Mozilla/5.0'})
    page = urllib.request.urlopen(req)
    return BeautifulSoup(page, "html.parser")


def clean_jobs(soup):
    # jobs are in 'spans'
    all_spans = soup.find_all("span")
    jobs = []
    for span in all_spans:
        jobs.append(span.get_text().strip())
    # remove extraneous elements
    jobs.remove('')
    jobs.remove('Subscribe to our EVENTS Newsletter')
    jobs.remove('Join our facebook GROUP')
    jobs = remove_value_from_list(jobs, '')
    return remove_value_from_list(jobs, 'new')


def organise(jobs):
    # make list of lists
    result = [["Title", "Company", "Location", "Duplicate", "Type", "Date Posted"]]
    new_list = []
    for job in jobs:
        if len(new_list) == 6:
            a = list(new_list)
            result.append(a)
            new_list = [job]
        else:
            new_list.append(job)
    result.append(new_list)
    return length_enforcer(result, 6)


def data_cleanser(result):
    for i in result:
        del i[3]
        try:
            i[4] = parser.parse(i[4])
        except ValueError:
            pass
    return result


def export(result):
    csvfile = "secret_today" + datetime.datetime.today().strftime('%m-%d') + ".csv"
    with open(csvfile, "w") as output:
        writer = csv.writer(output, lineterminator='\n')
        writer.writerows(result)


if __name__ == '__main__':
    update_db()
\$\endgroup\$
1
\$\begingroup\$

This could be used in clean_jobs, if you prefer list comprehension:

jobs = [span.get_text().strip() for span in all_spans]

export and final are reserved keywords in other languages, so they stand out a bit, to me. In either case, using more descriptive names for both could increase legibility.

The jobs.remove calls could be replaced with something like this (since more strings are almost guaranteed to appear in the future):

rem_list = ['',
            'Subscribe to our EVENTS Newsletter',
            'Join our facebook GROUP']
for removal_string in rem_list:
    jobs.remove(removal_string)

Adding # -*- coding: utf-8 -*- at the top is a healthy habit, for specifying that the source code is in UTF-8 (which is a good default).

Otherwise, I don't think the code looks too shabby. Code for scraping the web will never look squeaky clean, since there are many specifics to handle.

\$\endgroup\$
3
\$\begingroup\$

Code Quality and Other Improvements

  • I would use a cursor and a connection as context managers to avoid having to explicitly close them and let Python safely close them if an error happens:

    with sqlite3.connect('jobs.db') as connection:
        with connection.cursor() as cursor:
    
  • making the query defined as a multi-line string could benefit readability:

    cursor.executemany("""
        INSERT INTO 
            jobs 
        VALUES (?, ?, ?, ?, ?)""", new_result)
    
  • watch for PEP8 code style violations - for instance, organize imports in a proper way

Performance

We can definitely do better performance-wise:

  • switch from lxml to html.parser (requires lxml to be installed):

    BeautifulSoup(page, "lxml")
    
  • since you need only span elements, let BeautifulSoup parse only them:

    from bs4 import BeautifulSoup, SoupStrainer
    
    parse_only = SoupStrainer('span')
    BeautifulSoup(page, "lxml", parse_only=parse_only)
    
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for this feedback. I had some blips with the CM implementation, (the Python DBAPI precedes context managers so every RDBMS lib handles it differently, see here) but in the end I got it to work without a cursor with with sqlite3.connect('jobs.db') as con: con.executemany \$\endgroup\$ – Josh Friedlander Dec 19 '17 at 12:41

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.