Your code is pretty clean, you implement docstrings and it seems easy enough to read.
You do have 1 extra indentation level, which I will assume that it is a copy-paste-format error.
However...
Bugs
I found 2 bugs that are easy to catch.
getpage
returns strings
Here's the code in question:
# user defined functions
def getpage(query, url_type):
"""
A helper function to create a BeautifulSoup object. Input query string and specify the url type.
The goal is to return html code of a page.
query: str input. Accepts search query by "item name" or "asin"
url_type: specify the url type for the search you're doing. options = "item", "asin"
"""
if url_type == "item":
url = BASE_URL + "s?k=" + query
elif url_type == "asin":
url = BASE_URL + "product-reviews/" + query
else:
return "Error: url type unsupported. Choose from the following 'item', 'asin'"
response = requests.get(url, headers = HEADERS)
if response.status_code == 200:
return BeautifulSoup(response.text, "html.parser")
else:
return "Error: status_code != 200"
The first sentence of the docstring reads:
A helper function to create a BeautifulSoup object.
This is obviously a lie! The 6th and last lines return a string.
Return a BeautifulSoup or consider throwing a "proper" exception.
But why is this bad?
This is bad because you're using the return value directly.
Here's an example taken from get_prod_name
:
# [...]
html_raw = getpage(query, "item")
products = html_raw.find_all("span", class_ = "a-size-medium a-color-base a-text-normal")
# [...]
This will throw an exception because you're accessing the method find_all
on a string value. This method doesn't exist.
Input handling
More like, the lack of input handling...
You expect that the string is already URL-formatted.
Yeah, you do talk about this in the docstring, but, this is error prone and the reason why I classify it as a bug.
Just imagine trying to find HB #2 pencils with your code, and having to manually encode the space and the pound symbol.
You're putting the "user" in an error-prone situation because of pure lazyness.
All you have to do is to take the input and use requests.utils.quote
on it.
I found this on here: https://stackoverflow.com/questions/46783078/uri-encoding-in-python-requests-package#comment126847571_46783596
You just need to go to the functions get_prod_name
and get_asin
and add this as the first line:
query = requests.utils.quote(query, safe='')
But why add safe=''
?
As explained in the linked comment, this function assumes that /
is a safe character (as it is just the function urllib.parse.quote
).
Here's an example:
Leaving the bugs behind, now it's time to talk about ...
Weird things
Strange string usage
Inside the function get_prod_name
, you have the following:
query = query + f"&page={str(start)}"
Inside a formatted string, you don't need to use the str
function, as it is implicitly converted to string.
And then, you're concatenating it, which is kinda weird too.
You can just do this:
query = f"{query}&page={start}"
It will have the same exact effect.
Repeated code
You have quite a lot of repeated code, for apparently no reason at all...
Lets analyze the code inside the get_reviews
function:
# asin: list, start = 1, end = None
# [...]
if end == None:
query = list(map(lambda x: x + f"?pageNumber={str(start)}", asin))
html_raw = list(map(lambda x: getpage(x, "asin"), query))
html_tag = list(map(lambda x: x.find_all("span", attrs = {"data-hook": "review-body"}), html_raw))
html_tag = reduce(operator.iconcat, html_tag) # flatten the ResultSet
reviews = list(map(lambda x: x.text, html_tag))
reviews = [r.strip("\n") for r in reviews]
return reviews
else:
pages = range(start, end + 1)
queries = []
for p in pages:
for id in asin:
query = id + f"?pageNumber={str(p)}"
queries.append(query)
html_raw = list(map(lambda x: getpage(x, "asin"), queries))
html_tag = list(map(lambda x: x.find_all("span", attrs = {"data-hook": "review-body"}), html_raw))
html_tag = reduce(operator.iconcat, html_tag) # flattent the ResultSet
reviews = list(map(lambda x: x.text, html_tag))
reviews = [r.strip("\n") for r in reviews]
return reviews
So, what's the difference between the code in the ifs?
I compared both, and this is the result:
- The way the queries are built changes
- The way you call
getpage
- The spelling of "flatten" (vs. "flattent")
All the other lines are REPEATED, exactly byte by byte.
You can just write this:
if end == None:
html_raw = list(map(lambda x: getpage(f"{x}?pageNumber={start}", "asin"), asin))
else:
pages = range(start, end + 1)
queries = []
for p in pages:
for id in asin:
queries.append(f"{id}?pageNumber={p}")
html_raw = list(map(lambda x: getpage(x, "asin"), queries))
html_tag = list(map(lambda x: x.find_all("span", attrs = {"data-hook": "review-body"}), html_raw))
html_tag = reduce(operator.iconcat, html_tag) # flatten the ResultSet
reviews = list(map(lambda x: x.text, html_tag))
reviews = [r.strip("\n") for r in reviews]
return reviews
Yes, this can be better written, but this is just to give you an idea.
I also removed some (kinda) useless maps, but if you want to add them back, feel free to do so.
The strange pick of maps everywhere
On the above code, you do use the pattern list(map(lambda x: x))
a lot.
Try using list comprehensions, like this:
# old: html_raw = list(map(lambda x: getpage(x, "asin"), queries))
html_raw = [getpage(x, "asin") for x in queries]
Or, a better example:
# old: reviews = list(map(lambda x: x.text, html_tag))
# reviews = [r.strip("\n") for r in reviews]
reviews = [x.text.strip("\n") for x in html_tag]
This should work with BeautifulSoup, as the documentation contains the following example, in the first page:
One common task is extracting all the URLs found within a page’s tags:
for link in soup.find_all('a'):
print(link.get('href'))
# http://example.com/elsie
# http://example.com/lacie
# http://example.com/tillie
The name choices
There are some names that are extremely puzzling:
getpage
- gives you a BeautifulSoup object or a string
html_raw
- it's either a BeautifulSoup object or a string (no HTML here)
html_tag
- has a list of HTML elements, not tags
products
- it's either a list of HTML elements or a list of product names
Please, try to come up with better names.
There's a lot more to say about the code, but, I don't feel comfortable talking about those points.
I do have a limited python knowledge, and I just hope I've helped enough for others to pick and carry on from here (or even see any mistakes I've made and point them out to me as well).
Any questions, just throw them in the comments.