[SOLVED] Shortening near duplicate code?

This is the place for queries that don't fit in any of the other categories.

[SOLVED] Shortening near duplicate code?

Postby jeremyflexer » Sat Jan 18, 2014 1:16 pm

Hello all, I have a question about shortening code. It seems I run into this problem alot, I am new to python so it is hard for me to figure out how to shorten it.
I often find myself writing code like a few branches of if statements, and below it is almost the exact same thing save for variables and what I do with math. Below I will show you where I recently ran into this. I will try my best to fix it myself but I would really appreciate any help, thanks.
The method I ran into this at:
Code: Select all
    def movePlayer(self, key_pressed):
        self.key_pressed = key_pressed
        self.grid_copy[tuple(self.grid_player)] = self.grid_final[tuple(self.grid_player)]
        if (self.key_pressed == 'up') and (self.grid_player[0] > 1):
            if (self.grid_final[(self.grid_player[0] - 1, self.grid_player[1])] not in (self.wall['symbol'], self.rock['symbol'])):
                self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
                self.grid_player[0] = self.grid_player[0] - 1
            elif (self.grid_final[(self.grid_player[0] - 1, self.grid_player[1])] == self.rock['symbol']):
                self.message_move = "A rock is blocking your path."
            else:
                self.message_move = "A wall is blocking your path."
        elif (self.key_pressed == 'down') and (self.grid_player[0] < self.height):
            if (self.grid_final[(self.grid_player[0] + 1, self.grid_player[1])] not in (self.wall['symbol'], self.rock['symbol'])):
                self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
                self.grid_player[0] = self.grid_player[0] + 1
            elif (self.grid_final[(self.grid_player[0] + 1, self.grid_player[1])] == self.rock['symbol']):
                self.message_move = "A rock is blocking your path."
            else:
                self.message_move = "A wall is blocking your path."
        elif (self.key_pressed == 'left') and (self.grid_player[1] > 1):
            if (self.grid_final[(self.grid_player[0], self.grid_player[1] - 1)] not in (self.wall['symbol'], self.rock['symbol'])):
                self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
                self.grid_player[1] = self.grid_player[1] - 1
            elif (self.grid_final[(self.grid_player[0], self.grid_player[1] - 1)] == self.rock['symbol']):
                self.message_move = "A rock is blocking your path."
            else:
                self.message_move = "A wall is blocking your path."
        elif (self.key_pressed == 'right') and (self.grid_player[1] < self.length):
            if (self.grid_final[(self.grid_player[0], self.grid_player[1] + 1)] not in (self.wall['symbol'], self.rock['symbol'])):
                self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
                self.grid_player[1] = self.grid_player[1] + 1
            elif (self.grid_final[(self.grid_player[0], self.grid_player[1] + 1)] == self.rock['symbol']):
                self.message_move = "A rock is blocking your path."
            else:
                self.message_move = "A wall is blocking your path."
        else:
            self.message_move = "A wall is blocking your path."
        self.grid_final[tuple(self.grid_player)] = self.player['symbol']
Last edited by Mekire on Sat Jan 18, 2014 2:58 pm, edited 3 times in total.
Reason: first post initial lock
User avatar
jeremyflexer
 
Posts: 23
Joined: Fri Jan 17, 2014 6:36 pm

Re: Shortening near duplicate code?

Postby metulburr » Sat Jan 18, 2014 1:37 pm

this is not perfect as i have no way of testing the code without a massive implementation on my part, but basically you can shorten movement code by changing a movement value as positive or negative, then adding that value to the position. something like:
Code: Select all
...
   if (self.key_pressed == 'up') and (self.grid_player[0] > 1):
      movement = 1
   elif (self.key_pressed == 'down') and (self.grid_player[0] < self.height):
      movement = -1
...

Code: Select all
...
   if (self.grid_final[(self.grid_player[0] + movement, self.grid_player[1])] not in (self.wall['symbol'], self.rock['symbol'])):
      self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
      self.grid_player[0] = self.grid_player[0] + movement
   elif (self.grid_final[(self.grid_player[0] + movement, self.grid_player[1])] == self.rock['symbol']):
      self.message_move = "A rock is blocking your path."
   else:
      self.message_move = "A wall is blocking your path."
...

The movement variable would essentially be the Y axis
your ultimate goal is to make it not be like you copied and pasted it. In that way, if you have to change a value, you chagne one value and not 4 values. Plus it makes the code easier to read, simpler, and shorter.
New Users, Read This
OS Ubuntu 14.04, Arch Linux, Gentoo, Windows 7/8
https://github.com/metulburr
steam
User avatar
metulburr
 
Posts: 1407
Joined: Thu Feb 07, 2013 4:47 pm
Location: Elmira, NY

Re: Shortening near duplicate code?

Postby jeremyflexer » Sat Jan 18, 2014 1:41 pm

oh I see, I wonder if I could make 2 variables for movement, movement_one and movement_two, to handle self.grid_player[0] and [1]. so if I need to subtract I can just make the movement variable a negative right? That is what you said, and I think that would work perfectly, thanks man.
Last edited by Mekire on Sat Jan 18, 2014 1:48 pm, edited 2 times in total.
Reason: Quote snip.
User avatar
jeremyflexer
 
Posts: 23
Joined: Fri Jan 17, 2014 6:36 pm

Re: Shortening near duplicate code?

Postby Mekire » Sat Jan 18, 2014 1:43 pm

Untested as you didn't provide full code but something like this.

Create a constant dictionary in the global scope. Then use that for all movements:
Code: Select all
DIRECT_DICT = {"up"    : (-1, 0),
               "down"  : ( 1, 0),
               "left"  : ( 0,-1),
               "right" : ( 0, 1)}


def movePlayer(self, key_pressed):
    self.key_pressed = key_pressed
    position = tuple(self.grid_player)
    self.grid_copy[position] = self.grid_final[position]
    vec = DIRECT_DICT[self.key_pressed]
    new_pos = [self.grid_player[0]+vec[0], self.grid_player[1]+vec[1]]
    if (1 < new_pos[0] < self.height) and (1 < new_pos[1] < self.length):
        next_cell = self.grid_final[tuple(new_pos)]
        if next_cell not in (self.wall['symbol'], self.rock['symbol']):
            self.grid_final[position] = self.floor['symbol']
            self.grid_player = new_pos
        elif next_cell == self.rock['symbol']:
            self.message_move = "A rock is blocking your path."
        else:
            self.message_move = "A wall is blocking your path."
    self.grid_final[tuple(self.grid_player)] = self.player['symbol']

Might be a mistake in there but hopefully you get the idea,
-Mek
User avatar
Mekire
 
Posts: 987
Joined: Thu Feb 07, 2013 11:33 pm
Location: Amakusa, Japan

Re: Shortening near duplicate code?

Postby jeremyflexer » Sat Jan 18, 2014 1:45 pm

Thanks so much guys, can't believe I didn't think of this before; Thanks to you guys I know how to shorten code like this :D.
Last edited by Mekire on Sat Jan 18, 2014 1:49 pm, edited 1 time in total.
Reason: Quote snip.
User avatar
jeremyflexer
 
Posts: 23
Joined: Fri Jan 17, 2014 6:36 pm

Re: Shortening near duplicate code?

Postby Yoriz » Sat Jan 18, 2014 2:03 pm

The other replys show how to improve the code, here just a note that might be usefull.
If you find yourself making methods with lots of nested if conditions you can break them up into there own methods to make them more readable.
Code: Select all
    def movePlayer(self, key_pressed):
        self.key_pressed = key_pressed
        self.grid_copy[tuple(self.grid_player)] = self.grid_final[tuple(self.grid_player)]
        if (self.key_pressed == 'up') and (self.grid_player[0] > 1):
            self.on_move_player_up()
        elif (self.key_pressed == 'down') and (self.grid_player[0] < self.height):
            self.on_move_player_down()
        elif (self.key_pressed == 'left') and (self.grid_player[1] > 1):
            self.on_move_player_left()
        elif (self.key_pressed == 'right') and (self.grid_player[1] < self.length):
            self.on_move_player_right()
        else:
            self.message_move = "A wall is blocking your path."
        self.grid_final[tuple(self.grid_player)] = self.player['symbol']

    def on_move_player_up(self):
        if (self.grid_final[(self.grid_player[0] - 1, self.grid_player[1])] not in (self.wall['symbol'], self.rock['symbol'])):
            self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
            self.grid_player[0] = self.grid_player[0] - 1
        elif (self.grid_final[(self.grid_player[0] - 1, self.grid_player[1])] == self.rock['symbol']):
            self.message_move = "A rock is blocking your path."
        else:
                self.message_move = "A wall is blocking your path."

    def on_move_player_down(self):
        if (self.grid_final[(self.grid_player[0] + 1, self.grid_player[1])] not in (self.wall['symbol'], self.rock['symbol'])):
            self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
            self.grid_player[0] = self.grid_player[0] + 1
        elif (self.grid_final[(self.grid_player[0] + 1, self.grid_player[1])] == self.rock['symbol']):
            self.message_move = "A rock is blocking your path."
        else:
            self.message_move = "A wall is blocking your path."

    def on_move_player_left(self):
        if (self.grid_final[(self.grid_player[0], self.grid_player[1] - 1)] not in (self.wall['symbol'], self.rock['symbol'])):
            self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
            self.grid_player[1] = self.grid_player[1] - 1
        elif (self.grid_final[(self.grid_player[0], self.grid_player[1] - 1)] == self.rock['symbol']):
            self.message_move = "A rock is blocking your path."
        else:
            self.message_move = "A wall is blocking your path."

    def on_move_player_right(self):
        if (self.grid_final[(self.grid_player[0], self.grid_player[1] + 1)] not in (self.wall['symbol'], self.rock['symbol'])):
            self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
            self.grid_player[1] = self.grid_player[1] + 1
        elif (self.grid_final[(self.grid_player[0], self.grid_player[1] + 1)] == self.rock['symbol']):
            self.message_move = "A rock is blocking your path."
        else:
            self.message_move = "A wall is blocking your path."
New Users, Read This
Join the #python-forum IRC channel on irc.freenode.net!
Spam topic disapproval technician
Windows7, Python 2.7.4., WxPython 2.9.5.0., some Python 3.3
User avatar
Yoriz
 
Posts: 782
Joined: Fri Feb 08, 2013 1:35 am
Location: UK

Re: Shortening near duplicate code?

Postby jeremyflexer » Sat Jan 18, 2014 2:21 pm

Haha thanks :p, I think I will try both ways and see which one I like better :P.
Last edited by Mekire on Sat Jan 18, 2014 2:59 pm, edited 1 time in total.
Reason: Quote snip.
User avatar
jeremyflexer
 
Posts: 23
Joined: Fri Jan 17, 2014 6:36 pm

Re: Shortening near duplicate code?

Postby jeremyflexer » Sat Jan 18, 2014 2:52 pm

I finished the final implementation thanks to you guys. I will mark the thread as solved if possible and below you can see the new method. Once again thank you for the help.

Code: Select all
    def movePlayer(self, key_pressed):
        self.key_pressed = key_pressed
        self.directions = {'up':(-1, 0), 'down':(1, 0), 'left':(0, -1), 'right':(0, +1)}
        self.grid_copy[tuple(self.grid_player)] = self.grid_final[tuple(self.grid_player)]
        if (self.grid_player[0] > 1):
            if (self.key_pressed in self.directions.keys()):
                if (self.grid_final[(self.grid_player[0] + self.directions[self.key_pressed][0],
                                     self.grid_player[1] + self.directions[self.key_pressed][1])]
                                     not in (self.wall['symbol'], self.rock['symbol'])):
                    self.grid_final[tuple(self.grid_player)] = self.floor['symbol']
                    self.grid_player = [self.grid_player[0] + self.directions[self.key_pressed][0],
                                        self.grid_player[1] + self.directions[self.key_pressed][1]]
                elif (self.grid_final[(self.grid_player[0] + self.directions[self.key_pressed][0],
                                       self.grid_player[1] + self.directions[self.key_pressed][1])]
                                       == self.rock['symbol']):
                    self.message_move = "A rock is blocking your path."
                else:
                    self.message_move = "A wall is blocking your path."
            else:
                self.message_move = "A wall is blocking your path."
            self.grid_final[tuple(self.grid_player)] = self.player['symbol']
            self.message_move = "You move to the {}".format(self.key_pressed)


EDIT: It seems I can't edit the first post so I can't put solved in the topic title.
User avatar
jeremyflexer
 
Posts: 23
Joined: Fri Jan 17, 2014 6:36 pm


Return to General Coding Help

Who is online

Users browsing this forum: Bing [Bot] and 2 guests