Optimizing Beautiful Soup elements

There are a couple things in this method that can be refactored to remove a lot of duplicate code:

1. Firstly, you have this if statement:

   if held_item.find_all('option')[1:]: ... else: ... 

   What this is really checking for is if there is more than 1 option found. So, instead of having to write nearly identical code to handle the cases of 0 options or at least 1 option, we can simply store the find_all result and modify it based on a quick length check:

   options = held_item.find_all('option') if len(options) > 1: options.pop(0) for option in options: append_dict(selection_container, option) # If there is a non-zero amount of spaces, there will be at least # 2 partitions when split. if option.text.count(' '): stock_info = {'message': reduce(lambda x, y: x + ' ' + y, option.text.split(' ')[1:])[1:-1], 'status': 'pre-order'} problem_dict.update({'stock': stock_info}) 

2. Now, take a look at this line of code:

   stock_info = { 'message': reduce(lambda x, y: x + ' ' + y, m.text.split(' ')[1:])[1:-1], 'status': 'pre-order' } 

   I have two points to make about this line of code:

   • As a quick disclaimer about this first point:

     The line is still compliant with PEP8, so this is just a personal preference.

     With that out of the way, I try to never to put an opening {/(/[ at the end of a line unless I have to. Also, I never put the closing }/)/] on a line by itself. So I would format the line like this:

     stock_info = {'message': reduce(lambda x, y: x + ' ' + y, m.text.split(' ')[1:])[1:-1], 'status': 'pre-order'} 

   • My second point about this line of code is about how you create message. They way you use reduce is not quite necessary. From what I can tell, you want to take everything after the first space, then everything of that section except the first and last characters. This can be easily done using the split function:

     stock_info = {'message': m.split(' ', 1)[-1][1:-1] ... 

     Or you can use list splicing:

     # Below is reduced version of: # m[m.index(' ') + 1:][1:-1] stock_info = {'message': m[m.index(' ') + 2:-1] ... 

Past these two points, there isn't much to improve the algorithm from what information you gave. If you wanted to reduce the number of lines, you could write the first for loop as a list comprehension:

holder = [moselect.find_all('div')[1:] for moselect in base_page.find_all('div', attrs={'class': 'Moselect'}) if moselect.find('div', attrs={'id': 'noSize'})] 

Style-wise, you could improve a few of your variable names (m, holder, and selection_container specifically) as some of them are either ambiguous or slightly misleading.

Here is your code with my suggestions implemented:

def get_info(): problem_dict = {} selection_container = [] holder = [moselect.find_all('div')[1:] for moselect in base_page.find_all('div', attrs={'class': 'Moselect'}) if moselect.find('div', attrs={'id': 'noSize'})] for held_item in holder[0]: options = held_item.find_all('option') if len(options) > 1: options.pop(0) for option in options: append_dict(selection_container, option) # If there is a non-zero amount of spaces, there will be at least # 2 partitions when split. if option.text.count(' '): stock_info = {'message': {'message': m[m.index(' ') + 2:-1], 'status': 'pre-order'} problem_dict.update({'stock': stock_info}) problem_dict.update(dict(selections=selection_container)) return problem_dict