[Tutor] Making Doubly Linked List with Less Lines of Code.

Steven D'Aprano steve at pearwood.info
Wed Dec 24 22:56:05 CET 2014


On Wed, Dec 24, 2014 at 04:35:06PM -0500, WolfRage wrote:
> I wrote some code recently to make a linked list of Nodes for a 2d 
> graph, so it consists of rows and columns. Now I wanted to make the code 
> support being doubly linked, forwards and backwards.  The difficult part 
> of this is that the links are per row and per column. But the code I 
> think is overly bloated. I am currently working on reducing the 
> complexity of it. If any one has the time to look at it, if you have 
> ideas for how I can re-write it to be much smaller I would appreciate 
> the information. If you need more code let me know, but I tried to 
> condense it since this singular function is around 325 lines of code. 

Wow. It certainly is bloated.

I don't have time to look at it in any detail right now, as it is 
Christmas Day here, but I'll give you a suggestion. Any time you find 
yourself writing more than two numbered variables, like this:

>         previous_col0_node = None
>         previous_col1_node = None
>         previous_col2_node = None
>         previous_col3_node = None
>         previous_col4_node = None
>         previous_col5_node = None
>         previous_col6_node = None
>         previous_col7_node = None

you should instead think about writing a list:

    previous_col_nodes = [None]*8

Then, instead of code like this:

>                     if col == 0:
>                         self.col0 = current_node
>                         previous_col0_node = current_node
>                     elif col == 1:
>                         self.col1 = current_node
>                         previous_col1_node = current_node
>                     elif col == 2:
>                         self.col2 = current_node
>                         previous_col2_node = current_node
etc.

you can just write:

    for col in range(number_of_columns):
        self.columns[col] = current_node
        previous_col_nodes[col] = current_node


Look for the opportunity to write code like this instead of using range:

    for col, the_column in enumerate(self.columns):
        self.columns[col] = process(the_column)


Any time you write more than a trivial amount of code twice, you should 
move it into a function. Then, instead of:

    if row == 0:
       if col == 0: a
       elif col == 1: b 
       elif col == 2: c
       elif col == 3: d 
       elif col == 4: e 
   elif row == 1:
       if col == 0: a
       elif col == 1: b 
       elif col == 2: c
       elif col == 3: d 
       elif col == 4: e 
   elif row == 3:
       # same again

you can write a function:

def process_cell(row, col):
       if col == 0: a
       elif col == 1: b 
       elif col == 2: c
       elif col == 3: d 
       elif col == 4: e 
    
# later on

for row in rows:
    for col in cols:
        process_cell(row, col)



Try those suggestions, and come back to us if you still need help.




-- 
Steven


More information about the Tutor mailing list