Web crawler that filters out non diseases

Question

It is very messy and I lack the experience to make it eloquent so I wanted some help with it. The process time is also very slow.

Currently it goes into the first page and goes through the links in the index looking for the word "symptoms." If its not found it will look for a link inside that link named "faqs" and go to that link and look for 'symptoms'.

import requests
import string
from bs4 import BeautifulSoup, Tag
alpha = 'a b c d e f g h i j k l m n o p q r s t u v w x y z'
alpha = alpha.split()
w = []
def disease_spider(maxpages):
    i = 0
    while i <= maxpages:


        url = 'http://www.cdc.gov/DiseasesConditions/az/'+ alpha[i]+'.html'
        source_code = requests.get(url)
        plain_text = source_code.text
        soup = BeautifulSoup(plain_text)

        for l in soup.findAll('a', {'class':'noLinking'}):
            if l.text != "USA.gov" and l.text != 'Inspector General' and l.text != '':
                href = l.get('href')                
                diseasechecker(href)
        i += 1
def diseasechecker(url2):
    source_code = requests.get(url2)
    plain_text = source_code.text
    soup = BeautifulSoup(plain_text)
    for y in soup.findAll('p'):
        x = str(y)
        txt = y.text
        if 'symptoms' in x and url2 not in w:
            w.append(url2)
        elif 'symptoms' in txt and url2 not in w:
            w.append(url2)
    for y in soup.findAll('li'):
        x = str(y)
        txt = y.text
        if 'symptoms' in x and url2 not in w:
            w.append(url2)
        elif 'symptoms' in txt and url2 not in w:
            w.append(url2)
    for y in soup.findAll('a'):
        href2 = y.get('href')

        try:
            s = str(href2)
            u = "http://www.cdc.gov"+s
        except UnicodeEncodeError:
            pass
        l3 = 0
        if 'faqs' in s:
            l3 = level3(u)
        if l3 == 1 and url2 not in w:
            w.append(url2)
def level3(url3):
    source_code = requests.get(url3)
    plain_text = source_code.text
    soup = BeautifulSoup(plain_text)
    for y in soup.findAll('p'):
        x = str(y)
        txt = y.text
        if 'symptoms' in x:
            return 1
        elif 'symptoms' in txt:
            return 1

disease_spider(0)
for i in range(len(w)):
    print w[i]

Answer 1

Here are some suggestions:

• More comments and docstrings would be nice, so the reader/reviewer can see what you intend the code to do (why was the code written this way?). It makes it easier to follow, because your intention is clear, and also then I can spot bugs where the implementation doesn't match the indentation.

• Add more whitespace! Particularly vertically; breaking up the functions with newlines will make it substantially easier to read/

• At the top of the file, you construct a list of lowercase letters alpha. You’ve also imported, but not used, the string module. It would be better to use string.ascii_lowercase, which works as follows:

  string.ascii_lowercase

  The lowercase letters 'abcdefghijklmnopqrstuvwxyz'. This value is not locale-dependent and will not change.

  Then you can get rid of the alpha variable, and just use this.

• disease_spider()

  You should use a for loop rather than a while loop:

  for i in range(maxpages):
      # do some stuff
  

  Rather than constructing the request URL using string concatenation, use new-style format strings:

  url = 'http://www.cdc.gov/DiseasesConditions/az/{}.html'.format(
      string.lowercase[i]
  )
  

  I’m guessing that this function goes through the pages from A to Z, yes? But there’s no guard against the user calling this function with maxpages ≥ 26, which would throw an IndexError here. You should add some sort of check.

  Use a better variable name than l in your soup.findAll loop. How about link?

  Then I’d have a list of forbidden names, and just check than link.text isn’t in that list. For example:

  forbidden_text = ['USA.gov', 'Inspector General', '']
  if link.text not in forbidden_text:
      # do some more stuff
  

• diseasechecker()

  I noticed when reading this function that you use this block of code in all three functions:

  source_code = requests.get(url)
  plain_text = source_code.text
  soup = BeautifulSoup(plain_text)
  

  You should consider pulling this out into its own function, that takes a URL and returns the corresponding BeautifulSoup object. It would slightly tidy up the code.

  The variable names in this function are all short and undescriptive; pick something that tells me what the value of this variable represents.

  This function mutates a global list w, although it’s the only function to do so. I think it would be better if you moved the definition of w inside this function, and returned it at the end. That makes the function more self-contained.

  You loop over all p and li elements separately, but do the same thing with each. Why not combine the loops, so you only have to write out the body of the loop once:

  for element in soup.findAll('p') + soup.findAll('li'):
       # do some stuff with elements
  

  I believe you can also tidy up the if statement:

  if (url2 not in w) and ('symptoms' in str(y) or 'symptoms' in y.text):
      w.append(url2)
  

  The parens aren’t strictly necessary, but I think they’re useful here for readability. I also got rid of the x and txt variables, which I don’t think aid readability. This is more direct.

• level3()

  Again, pick some better variable names that x and y.

  And you can tidy up the if statement:

  if 'symptoms' in str(y) or 'symptoms' in y.text:
      return 1
  

• In the final loop, you’re looping over the index:

  for i in range(len(w)):
      print w[i]
  

  It is far cleaner and more Pythonic to loop over the list directly; i.e.,

  for word in w:
      print word
  

Answer 2

First off, don't use while loops to execute a piece of code \$n\$ amount of times. You should be using for ... in range( ... ) loop instead. For example:

for _ in range(10):
    # Any code below here get's executed 10 times

Secondly, you have a ton of spacing inconsistencies. There should be two blank lines between each top-level function/class/code block. You have a few other style violations. For more information on how to properly style Python code, see PEP8.

Some of your variable names are pretty bad as well. A few examples of not so great names include m, y, x, l, l3, w, s, and u. Variable names should be long, but not too long, and as descriptive as possible.

Finally, you should be adding some docstrings to describe you functions. Docstrings should contain a description of the function/class, and it's arguments. Here's an example:

def my_func( ... ):
    """
    Add a description of your function
    and it's arguments in here.
    """