# Simple Web Crawler for Small Sites

This script is my web crawlerfor small sites. Can you possibly review it for best coding practices?

### Code

import bs4 as bs
import urllib.request
from urllib.parse import urlparse,urljoin
import pprint

class Page(object):
def __init__(self,base_url,url):
self.url = url
self.base_url = base_url
def soup(self):
return (bs.BeautifulSoup(sauce,'lxml'))
def title(self):
soup = self.soup()
return soup.title.string
urls = []
soup = self.soup()
href = [i.get('href') for i in soup.findAll('a') ]
links = [i for i in (list(map((lambda url : url if bool(urlparse(url).netloc) == True else urljoin (self.base_url, url)),href))) if i.startswith(self.base_url)]
def map_page(self):
return map

def site_map(base_url):
map_pages = {}

if url not in map_pages:
(map_pages.update(Page(base_url,url).map_page()))
else:

pprint.pprint(map_pages)

• Is the code working? If not it is off topic, please see how to ask a good question and what questions not to ask codereview.stackexchange.com/help/dont-ask. The part about "what I do wrong" leads to my question. – pacmaninbw Apr 18 '19 at 16:13
• Yes code is working . I mean what is the best practices – Dawid Apr 18 '19 at 16:26

I have made some changes to make it neater and more efficient. See the end of this response for the final solution. But I'll start with a few comments.

There is no point making your class inherit the object class, especially if you're working with Python 3.

class Page(object):


The soup() method seems to be called indiscriminately causing BeautifulSoup to parse the contents of the page in concern too many times (see the map_page() method, for example). It is probably more efficient to parse once, afterwhich you can save whatever (e.g. title and links)you need as object properties.

Using list and map in this context seems unnecessary. It helps if you strive for readability --- your future self will thank you.

links = [i for i in (list(map((lambda url : url if bool(urlparse(url).netloc) == True else urljoin (self.base_url, url)),href))) if i.startswith(self.base_url)]


Also, checking the truthiness of urlparse(url).netloc doesn't require you to coerce it to a boolean result first. Simply replacing if bool(urlparse(url).netloc) with if urlparse(url).netloc would suffice.

In the site_map() function, you can replace the while loop with something simpler, e.g.:

while links_to_map:


I have also made other subjective simplifications that hopefully should be obvious to you.

import pprint
import urllib.request
from pprint import pprint
from urllib.parse import urlparse, urljoin

import bs4 as bs

class Page:
def __init__(self, base_url, url):
self.url = url
self.base_url = base_url
self.souped = None
self.title = None

def soup(self):
def clean(url):
return url if urlparse(url).netloc else urljoin(self.base_url, url)

self.souped = bs.BeautifulSoup(sauce, "lxml")
self.title = self.souped.title.string
hrefs = set([clean(i.get("href")) for i in self.souped.findAll("a")])
return self

@property
def map_page(self):
return lookup

def site_map(base_url):
map_pages = {}

if url not in map_pages:
page = Page(base_url, url).soup()
map_pages.update(page.map_page)

pprint(map_pages)


not exhaustive but some feedback. There are some big "No No"s in this code as well as some minor formatting things.

First the big "No No"s.

1) Do not use list comprehension unless you plan do use the result: [links_to_map.append(i) for i in Page(base_url,url).links()] Do this in a normal for loop.

2) DO NOT change a list you are iterating over (i.e. links_to_map in your lest method).

Other stuff:
return statements don't need ():

return (bs.BeautifulSoup(sauce,'lxml'))

Should just be:
return bs.BeautifulSoup(sauce,'lxml')

Avoid needless () as well:

(map_pages.update(Page(base_url,url).map_page())

to:
map_pages.update(Page(base_url,url).map_page()

Empty lists are considered False so:

while links_to_map != []:

can just be:
while links_to_map:

My last tip. Consider using a set for links_to_map instead of a list since the removing step will be significantly faster.

• (Congrats on your first post here at long last…) – greybeard Apr 18 '19 at 20:07