3
\$\begingroup\$

As if this isn't extremely obvious, I am a new coder.

My code works, but is quite far from ideal. I'm also not sure if there are unnecessary bits, since I've been trying to get this to work for a few days on and off, so I've started and stopped multiple times.

The goal is to have cleaner code, with a #comment on each line (unless extremely basic) so as to improve my annotation habits. Please let me know what you would do to improve it overall.

import urllib.request as ur
from bs4 import BeautifulSoup


url = str(input('Enter URL- ')) #convert input to string
html = ur.urlopen(url).read() #read html
soup = BeautifulSoup(html, "html.parser") #retrieve all of the anchor tags
Count_ = int(input('Enter count: ')) #convert input to integer
pos_1 = int(input('Enter position: ')) #convert input to integer
tags = soup('a')
final = '' #url of name list before break
curpos = ''
print('Retrieving: ', url) #prints starting point/url
count = int(Count_) + 1
while count > 1 : #starting a definite loop that goes until count is smaller than 1
    pos = 0
    for tag in tags :
        if pos == int(pos_1) - 1 : #conditional statement regarding position
            curpos = tag.get('href', None)
            break
        pos = pos + 1 #increases value of pos for each tag
    final = curpos #
    url = str(curpos) #
    html = ur.urlopen(url).read()
    soup = BeautifulSoup(html, "html.parser")
    tags = soup('a')
    count = count - 1 #for every iteration in the loop, subtract 1 from the value of count
    print('Retrieving: ', final)
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

General Observations

  • instead of manually looking for a tag a desired position and handling pos increment in the loop, I think you could just simply get the value by index:

    curpos = tags[int(pos_1) - 1].get('href', None)
    
  • count = count - 1 could be simplified as count -= 1

  • follow the PEP8 lower_case_with_underscores variable naming guideline
  • what if you prefix the variable names containing user-defined values with input_ (see below)?
  • and, I think a "for" loop with a negative step would be a simpler solution that the while loop here:

    for count in range(int(Count_) + 1, 1, -1):
        # ...
    
  • Or, to bring it one step further, what if we apply a generally easier to follow recursive flow instead of the iterative approach you currently have. The base condition for the recursion could be the input count reaching 1. And, we'll improve on DRY with that function as well.

Web-Scraping

  • you don't have to call .read() on the result of .urlopen() as BeautifulSoup also accepts file-like objects:

    soup = BeautifulSoup(ur.urlopen(url), "html.parser")
    
  • switching from html.parser to lxml may help drastically improve HTML-parsing performance

  • instead of using urllib(), you could switch to requests and re-use a session which would help avoid an overhead of re-establishing network connection to the host on every request
  • you could use SoupStrainer to let BeautifulSoup parse only the a elements
  • you should also account for relative links and use urljoin() to combine base urls and relative links

The code with the above and other improvements applied:

from urllib.parse import urljoin

import requests
from bs4 import BeautifulSoup, SoupStrainer


only_links = SoupStrainer('a')


def follow_link(session, url, position, count):
    """Follows a link at a given "position" "count" number of times."""
    if count <= 1:
        return

    print('Retrieving: ', url)
    response = session.get(url)

    soup = BeautifulSoup(response.content, "lxml", parse_only=only_links)
    links = soup('a')
    next_url = links[position - 1].get('href', None)

    return follow_link(session, urljoin(url, next_url), position, count - 1)


if __name__ == '__main__':
    input_url = str(input('Enter URL- '))
    input_count = int(input('Enter count: '))
    input_position = int(input('Enter position: '))

    with requests.Session() as session:
        follow_link(session, input_url, input_position, input_count)

Afterthoughts

  • what if there is no link at the desired position available on some page?
  • if we once get a link which links itself, this code in this state would get stuck on this page alone until the count is exchausted

The goal is to have cleaner code, with a #comment on each line (unless extremely basic) so as to improve my annotation habits.

Comments on each line could overall decrease readability of the code and they are, in essence, extra information and weight you need to make sure stay up to date with the actual code. Self-documented code is something you should strive to achieve, using comments as an extra measure used to explain the "why" some decisions were made and not the "how".

\$\endgroup\$
1
  • \$\begingroup\$ "Self-documented code is something you should strive to achieve, using comments as an extra measure used to explain the "why" some decisions were made and not the "how"." This is very helpful in my process, thank you! \$\endgroup\$
    – Jon
    Commented Jan 27, 2019 at 16:33

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.