2
\$\begingroup\$

This code extracts marks of all students in class and stores the result in a file results.txt, using BeautifulSoup. I'm looking for code review and suggestions.

from bs4 import BeautifulSoup
import requests

file = open("results.txt","w")

for roll in range(1,60):
    if roll in range(1,10):
            rollstr = '0'+str(roll)
    else:
            rollstr = str(roll)
    url = 'http://www.*.org/'
    r = requests.get(url)

    data = r.text
    soup = BeautifulSoup(data)

    file.write('roll: ' +rollstr + ' ')

    for i in soup.find_all('strong'):
            sgpa = i.get_text()
            if sgpa.find('SGPA') == 0:
                    file.write(sgpa)
                    break
    file.write('\n')
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Style

Your code looks good but you might like to know that there is a Style Guide for Python Code called PEP 8. If you are interested, you'll find tools to check your code automatically (pep8) and to fix it (autopep8).

In your case, I can see straight-away that you are sometimes missing spaces around your '+' operator.

String format

You'll find various more pythonic ways to format your number with leadings 0s.

Also, in case you want to keep your current logic, comparison can be written pretty succintly in Python so that you can simply write : if 1 <= roll < 10. (Please note that in Python 2.x, range returns a list so checking if x in range(whatever) is not efficient in time (it has to check all elements if the elements you are looking for is not in the list) nor in memory (a list is built for no reason)).

Using context managers

Opening files and remembering to close them is a bit tedious. Good thing is that Python has a clean solution to this : the with keyword. In a nutshell, you use with when you open your file, you add a level of indentation and the magic to close your file will happen automaticaly.

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

Couple web-scraping and HTML-parsing related notes:

  • since you are issuing multiple requests and to the same domain, instantiate a web-scraping requests.Session object and re-use to make requests - this would provide a performance boost since all the requests in the same session would re-use the same TCP connection to the host
  • when you instantiate a BeautifulSoup class, you should specify what parser BeautifulSoup needs to be using under-the-hood:

    soup = BeautifulSoup(response.text, "html.parser")
    # soup = BeautifulSoup(response.text, "lxml")
    # soup = BeautifulSoup(response.text, "html5lib")
    

    Otherwise, it would pick one automatically based on the installed packages in this particular Python environment. This may lead to BeautifulSoup picking up different parser on different machines or Python environments which would make your script unstable, behaving differently on different machines. See more at Installing a Parser and Differences Between Parsers

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.