Battleship coordinate picker

I present to you… my battleship location picker! This will be part of a larger battleship program. The coordinates that are important are finalcoords.

from operator import itemgetter
import getch
import skilstak.colors as c #colors
BOARDLENGTH = 10
BOARDHEIGHT = 10
board = [['_'] * BOARDLENGTH for _ in range(BOARDHEIGHT)]
class Ship():
    def __init__(self, size, symbol):
        self.size = size
        self.symbol = symbol
        self.coords = [(x,0) for x in range(size)]
        self.previous_coords = [(None,None) for _ in range(size)]
    def shift(self,direction):

        x_values = list(map(itemgetter(0), self.coords))
        y_values = list(map(itemgetter(1), self.coords))
        if direction == 'R' and 0 not in x_values:
            change = (-1,0)    
        elif direction == 'D' and BOARDHEIGHT-1 not in y_values:
            change = (0,1)
        elif direction == 'U' and 0 not in y_values:
            change = (0,-1)
        elif direction == 'L' and BOARDLENGTH-1 not in x_values:
            change = (1,0)
        else:
            change = (0,0)
        for i, coord in enumerate(self.coords):
            self.previous_coords[i] = self.coords[i] 
            self.coords[i] = (coord[0] + change[0], coord[1] + change[1])

    def rotate(self):
        a, b = self.coords[0][0], self.coords[0][1]
        x, y = self.coords[-1][0], self.coords[-1][1] 
        newx, newy = -(y-b)+a, (x-a)+b
        if newy >= 0 and newx >= 0 and newy <= BOARDHEIGHT-1 and newx <= BOARDLENGTH-1:
            for i, coord in enumerate(self.coords):
                self.previous_coords[i] = self.coords[i]
                self.coords[i] = (-(self.coords[i][1]-b)+a, (self.coords[i][0]-a)+b)

    def show(self, board, hasprevious=True):
        if hasprevious:
            for pcoord in self.previous_coords:
                board[pcoord[1]][pcoord[0]] = '_'
        for coord in self.coords:
            board[coord[1]][coord[0]] = self.symbol
        display_board = 'n'.join(' '.join(line) for line in board)
        print(c.clear + display_board) #c.clear clears the screen
        return board
def hide(board,coords):
    for coord in coords:
        board[coord[1]][coord[0]] = '_'
    return board

def choose_locations(board):
    ships = [Ship(5,'A'),Ship(4,'B'),Ship(3,'c'),Ship(3,'C'),Ship(2,'D')] 
    finalcoords = []
    for ship in ships:
        board = ship.show(board,hasprevious=False)
        while True:
            key = getch.getch()
            if key == 'w':
                ship.shift('U')
                board = ship.show(board)
            elif key == 's':
                ship.shift('D') 
                board = ship.show(board)
            elif key == 'a':
                ship.shift('R') 
                board = ship.show(board)
            elif key == 'd':    
                ship.shift('L') 
                board = ship.show(board)
            elif key == 'r':
                ship.rotate()
                board = ship.show(board)
            elif key == 'q':
                exit()
            elif key == 'r':
                intersection = list(set(ship.coords).intersection(finalcoords))
                if intersection == []:
                    finalcoords += ship.coords
                    board = hide(board,ship.coords)
                    break
                else:
                    print('''You already have a ship or part of a ship there.Your current coordinates are {}
You have placed a ship on these coords, {}. 
You overlapped on these coords: {}'''.format(ship.coords,finalcoords,intersection))
            else:
                print('press w,a,s or d to move, and r to rotate. Press q to quit.')
    return finalcoords
if __name__ == '__main__':                
    coords = choose_locations(board)

Here are some observations:

• no meaningful comments or docstrings (they are not always needed, but imagine there is that “violent psychopath, who knows where you live” will be reading the code)

• PEP8 violations and making things Pythonic:

  • blank line between class methods, two blank lines between top-level functions and classes (source)
  • correct import organization
  • the inline comments formatting

  • variable naming, if defining a constant, there has to be an underscore between words:

    BOARDLENGTH -> BOARD_LENGTH
    BOARDHEIGHT -> BOARD_HEIGHT
    

  • a space between arguments after a comma:

    ship.show(board, hasprevious=False)
                HERE^
    

  • no need for extra () after the class definition
  • if intersection == []: can be rewritten as if not intersection:

  • simplify the if condition inside the rotate() method, replace:

    if newy >= 0 and newx >= 0 and newy <= BOARDHEIGHT-1 and newx <= BOARDLENGTH-1:
    

    with:

    if 0 <= newx <= BOARDLENGTH - 1 and 0 <= newy <= BOARDHEIGHT - 1:
    

• other improvements:

  • looks like you don’t need to make the size an instance variable – you only need it in the constructor

  • what if instead of multiple repetitive if/else blocks inside the choose_locations(), you would create a mapping between action keys and the shift directions, e.g.:

    ACTION_KEYS = {
        'w': 'U',
        's': 'D',
        'a': 'R',
        'd': 'L'
    }
    

    Then, we can improve the part determining the shift direction:

    key = getch.getch()
    if key in ACTION_KEYS:
        ship.shift(ACTION_KEYS[key]) 
        board = ship.show(board)