HackerRank “Bot Saves Princess” beginner code

Question

I submitted the following Python 3 code for this HackerRank challenge. Given an n × n grid with an m in the center and a p in one corner, it prints the directions to move the m to the p.

I wanted to know how improve it or other observations.

#!/usr/bin/python
def displayPathtoPrincess(n,grid):
    #first, find where p is located 
    flag = True
    n = n-1
    while(flag):
        if grid[0][n] == 'p':
            flag = False 
            a = 0
            b = n
        if grid[n][0] == 'p':
            flag = False 
            a = n
            b = 0
        if grid[0][0] == 'p':
            flag = False 
            a = 0
            b = 0 
        if grid[n][n] == 'p':
            flag = False 
            a = n
            b = n 
        #else:
            #print("Something broke?") #Why does this execute?
        y = a - int(n/2)
        x = b - int(n/2)    
        while 0 != y:
            if y < 0:
                y = y+1
                print("UP")
            if y > 0:
                y = y-1
                print("DOWN")
        while 0 != x:
            if x <0:
                x = x+1
                print("LEFT")
            if x >0:
                x = x-1
                print("RIGHT")



m = int(input())
grid = [] 
for i in range(0, m): 
    grid.append(input().strip())

displayPathtoPrincess(m,grid)

Answer 1

The function should be path_to_princess(grid), using the naming convention specified in PEP 8. The n parameter is redundant, as it can be inferred from the grid's dimensions. In any case, redefining n=n-1 is confusing. Also, it would be better to leave all I/O out of the function, so that it just does computation.

The while(flag) loop shouldn't exist. If the princess is in another castle, you'll end up with an infinite loop; otherwise, the code executes just once.

Use the // operator for integer division.

Explicit looping can be greatly reduced through liberal use of list comprehensions, generators, and list multiplication.

def path_to_princess(grid):
    z = len(grid) - 1
    corners = [
        (0, 0), (0, z),
        (z, 0), (z, z)
    ]
    p = next((r, c) for r, c in corners if grid[r][c] == 'p')
    m = (z // 2, z // 2)
    row_diff, col_diff = p[0] - m[0], p[1] - m[1]    
    yield from (['UP'] * -row_diff if row_diff < 0 else ['DOWN'] * row_diff)
    yield from (['LEFT'] * -col_diff if col_diff < 0 else ['RIGHT'] * col_diff)

n = int(input())
grid = [input().strip() for _ in range(n)]
for move in path_to_princess(grid):
    print(move)

Answer 2

I would start by losing flag. All it does is keep looping if the princess isn't found. Does this really make sense?

Consider consolidating your logic and using the built in print functionality. In pseudo code, you might have something like this:

horizontalDistance = princess Location X- start position
verticalDistance = princess location Y - start position

You can then use the print functionality to print a string multiple times using the absolute value of the distance. Something like this:

print(("LEFT" if horizontalDistance > 0 else "RIGHT") * abs(horizontalDistance))

You could obviously use a variable, rather than the ternary operator for readability.

Answer 3

1. Remove the debugging code (else how did I get here)
2. This should have been 2 functions, the first is find the princess. The second is print the path to the princess.