Your code is pretty clean, you implement docstrings and it seems easy enough to read.<br> 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: ```python # 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:<br> > 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`: ```python # [...] 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. <hr> ## 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.<br> 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.<br> 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: ```python 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`][1]). Here's an example: [![Example of using the function requests.utils.quote][2]][2] <hr> <hr> 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: ```python 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: ```python query = f"{query}&page={start}" ``` In this case, maybe you should just use the `+=` operator: ```python query += f"&page={start}" ``` Both 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: [![Result of comparing both ifs][3]][3] - 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: ```python 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 ``` I also removed some (kinda) useless maps, but if you want to add them back, feel free to do so. However, it can be reduced a little more, if you're okay with changing variables as well: ```python if end == None: queries = list(map(lambda x: f"{x}?pageNumber={start}", asin)) else: queries = [] for page in range(start, end + 1): for id in asin: queries.append(f"{id}?pageNumber={page}") 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 ``` I've removed the variables `query` and `pages`, and reduced a repeated line. I'm sure there's better ways to re-write this, but this is just to get your feet wet and a few ideas on what to do. ## 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: ```python # old: html_raw = list(map(lambda x: getpage(x, "asin"), queries)) html_raw = [getpage(x, "asin") for x in queries] ``` Or, a better example: ```python # 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][4] contains the following example, in the first page: > One common task is extracting all the URLs found within a page’s <a> tags: > ```python > 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. <hr> <hr> 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. [1]: https://docs.python.org/3/library/urllib.parse.html#urllib.parse.quote [2]: https://i.sstatic.net/Ol3JC.png [3]: https://i.sstatic.net/9wRPz.png [4]: https://www.crummy.com/software/BeautifulSoup/bs4/doc/