3
\$\begingroup\$

This code is scraping the data from IMDB and displays it in JSON or CSV format.

I plan to extend it beyond the top rated movies. How can I improve the quality of my code? I am using Python 3.7.3.

from bs4 import BeautifulSoup
import requests
import csv
import json
import codecs

films = []

def getFilms250():
    sourceTop250 = requests.get(
        'https://www.imdb.com/chart/top?ref_=nv_mv_250').text
    soup = BeautifulSoup(sourceTop250, 'lxml')
    counter = 0
    for title_container, rating_container in zip(soup.find_all('td', class_='titleColumn'),soup.findAll('td', class_='ratingColumn imdbRating')):
        title = title_container.a.text
        counter += 1
        year = title_container.span.text
        rating = rating_container.strong.text
        film_data = [counter, title, year.strip('()'), rating]
        films.append(film_data)
    return films


def csvFormat(films, userDelimiter):
    with open('new_film_base.csv', 'w', newline='') as file:
        header = ['Position', 'Title', 'Year', 'imdbRanking']
        film_writer = csv.writer(file, delimiter=userDelimiter)
        film_writer.writerow(header)
        film_writer.writerows(films)
    file.close


def jsonFormat(films):
    with open('new_film_base.json', 'w', encoding='utf8') as file:
        arr = []
        for i in films:
            x = {
                "position": i[0],
                "title": i[1],
                "year": i[2],
                "imbdRanking": i[3]
            }
            arr.append(x)
        movies = {
            "filmsRequested": arr
        }
        json.dump(movies, file, indent=True, ensure_ascii=False)
    file.close


def handleUserFilm(prompt):
    #While user enters a proper input a function will get list of wanted films
    while True:
        try:
            userMessageFilm = input((prompt))
        except ValueError:
            print('Input unkown')
        if userMessageFilm == 'films250':
            getFilms250()
            break
        elif userMessageFilm != 'films250':
            print('No such data')
            continue
    return userMessageFilm


def handleUserFormat(prompt):
    while True:
        try:
            userMessageFormat = input((prompt))
        except ValueError:
            print('Input unkown')
            continue
        if userMessageFormat != 'csv' and userMessageFormat != 'json':
            print('Format unkown')
            continue
        elif userMessageFormat == 'json':
            jsonFormat(films)
            break
        elif userMessageFormat == 'csv':
            while True:
                delimiter = input(
                    'Enter the Delimiter for csv format(";" recommended): ')
                if len(delimiter) > 4:
                    print("delimiter can't be that long")
                    continue
                else:
                    csvFormat(films, delimiter)
                    break

        return userMessageFormat


filmsUser = handleUserFilm('To get Top250 type - films250: ')
formats = handleUserFormat('To get csv format type - csv\nTo get json format type - json ')
\$\endgroup\$
  • \$\begingroup\$ Hi Philip, I suggest you edit your title to state what your code does in more detail, for example "Scraping IMDB and exporting to JSON or CSV", this way your question will have more attention! :) \$\endgroup\$ – IEatBagels Nov 13 '19 at 21:05
5
\$\begingroup\$

Python uses snake_case, not camelCase for variable and function names. csvFormat for example, should be csv_format.


In two places you do this weird thing:

with open(. . .) as file:
    . . .
file.close
  • with already closes file, so there's no need to manually close it.

  • file.close doesn't do anything, and a good IDE would warn you of that. You need to add () to call close: file.close().


Speaking of weird things, you have this in a couple places as well:

try:
    userMessageFormat = input((prompt))
except ValueError:
    print('Input unkown')

input, as far as I know, will never throw. If you had a call to int in there, or if you were using Python 2 (maybe?), this may make sense. I don't think this try does anything, and I don't think 'Input unkown' will ever be printed.


if userMessageFormat != 'csv' and userMessageFormat != 'json':
    . . .
elif userMessageFormat == 'json':
    . . .
elif userMessageFormat == 'csv':

This is written in kind of a "fragile" way. If you ever add more formats, you'll need to make two changes: 1. Add a new elif userMessageFormat == check, and 2. Add to the first condition to validate the format ahead of time. If you forget to update the latter, none of the other cases will be True, and return userMessageFormat will cause a NameError since userMessageFormat will have never been defined.

Just make use of an else case:

if userMessageFormat == 'json':
    . . .
elif userMessageFormat == 'csv':
    . . .
else:
    print('Format unkown')

This is what else is for: to handle when none of the other checks were true.


for i in films:
    x = {
        "position": i[0],
        "title": i[1],
        "year": i[2],
        "imbdRanking": i[3]
    }

This could make use of destructuring to avoid needing to index i:

for position, title, year, rank in films:
    x = {
        "position": position,
        "title": title,
        "year": year,
        "imbdRanking": rank
    }

If the sub-collections in films contain more than 4 elements each, you'll need to change the for line to:

for position, title, year, rank, *_ in films:

The *_ catches all the remaining elements. Without this, you'll get a ValueError.


if userMessageFilm == 'films250':
    . . .
elif userMessageFilm != 'films250':
    . . .

This is redundant. If the first check is false, the second must be true. This can just be:

if userMessageFilm == 'films250':
    . . .
else:
    . . .
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thank you for your answear. Some of those mistakes were really stupid. I'll upadate my code considering your suggestions \$\endgroup\$ – Philip Nov 13 '19 at 23:30
  • 1
    \$\begingroup\$ @Philip You're welcome. Note though, don't update the code in the question here (I'm not sure of that's what you meant). Code being reviewed isn't generally allowed to be edited once it's posted, as that invalidates answers. \$\endgroup\$ – Carcigenicate Nov 13 '19 at 23:34

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.