0
\$\begingroup\$

I'm programming a board game called Djambi, where most of its pieces can move like the queen from chess. I created a list to save all valid destinations of a piece, which can be in 8 different directions, so I used 8 for loops, but the code of each loop is almost identical:

This is the loop for possible movements to the right:

count = 0
for j in range(y+1,9):
    if P == 3 and count == 2:
        break

    if re.search(r'[■]',board[x][j]):
        pos = str(x)+str(j)
        destinations.append(pos)
    elif re.search(r'\d', board[x][j]):
        if re.search(r'' + chip[0], board[x][j]) or P == 1 or P == 0:
            break
        else:
            pos = str(x)+str(j)
            destinations.append(pos)
            break
    elif re.search(r'[░]',board[x][j]):
        if P == 0:
            pos = str(x)+str(j)
            destinations.append(pos)
            break
        else:
            break
    else:
        if P == 2:
            pos = str(x)+str(j)
            destinations.append(pos)
        else:
            continue

    count += 1

The loop for movements to the left can be simplified with the one above, but the problem is for the loops for movements up

count = 0
for i in range(x+1,9):
    if P == 3 and count == 2:
        break

    if re.search(r'[■]',board[i][y]):
        pos = str(i)+str(y)
        destinations.append(pos)
    elif re.search(r'[\d]',board[i][y]):
        if re.search(r'' + chip[0], board[i][y]) or P == 1 or P == 0:
            break
        else:
            pos = str(i)+str(y)
            destinations.append(pos)
            break
    elif re.search(r'[░]',board[i][y]):
        if P == 0:
            pos = str(i)+str(y)
            destinations.append(pos)
            break
        else:
            break
    else:
        if P == 2:
            pos = str(i)+str(y)
            destinations.append(pos)
        else:
            continue

    count += 1

and down, because the order of variables that change is different, also for diagonal movements:

count = 0
for i,j in zip( range(x+1,9) , range(y+1,9) ):
    if P == 3 and count == 2:
        break

    if re.search(r'[■]',board[i][j]):
        pos = str(i)+str(j)
        destinations.append(pos)
    elif re.search(r'[\d]',board[i][j]):
        if re.search(r'' + chip[0], board[i][j]) or P == 1 or P == 0:
            break
        else:
            pos = str(i)+str(j)
            destinations.append(pos)
            break
    elif re.search(r'[░]',board[i][j]):
        if P == 0:
            pos = str(i)+str(j)
            destinations.append(pos)
            break
        else:
            break
    else:
        if P == 2:
            pos = str(i)+str(j)
            destinations.append(pos)
        else:
            continue

    count += 1
  • x and y are the fixed coordinates of the original position of the chip
  • i and j are the variable coordinates
  • board is a matrix of 9x9 dimensions

I want to know if there is a way to implement a kind of function of method or whatever Python can provide to simplify the code above in one chunk, and call it depending of what is needed, in this case, calling it 8 times.

The code works, I just want to know how to simplify it and make it more understandable.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! What task does this code accomplish? Please tell us, and also make that the title of the question via edit. Maybe you missed the placeholder on the title element: "State the task that your code accomplishes. Make your title distinctive.". Also from How to Ask: "State what your code does in your title, not your main concerns about it.". \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Sep 27 '18 at 21:38
  • \$\begingroup\$ Can you explain what P is? \$\endgroup\$ – 200_success Sep 27 '18 at 22:05
  • \$\begingroup\$ @200_success P is the current piece. I use that variable because some pieces stops at alive piece, one at dead piece, some only can move 2 squares, etc. \$\endgroup\$ – netotz Sep 27 '18 at 23:20
  • \$\begingroup\$ I'd work on creating some tests first, before attempting to refactor the code. There are too many conditionals in each loop to ensure your future changes will actually give you the results you expect without introducing bugs. \$\endgroup\$ – C. Harley Sep 28 '18 at 5:58
0
\$\begingroup\$

Yes, you can simplify the implementation of your algorithm by introducing the notion of direction, like you find in mathematics…

In Djanbi game, a token can move in 8 directions. You can define a vector d(d_x, d_y) which defines this notion of direction:

  1. North: d = (0, 1),
  2. East: d = (1, 0),
  3. South: d = (0, -1),
  4. West: d = (-1, 0),
  5. North-East: d = (1, 1),
  6. South-East: d = (1, -1),
  7. North-West: d = (-1, 1),
  8. South-West: d = (-1, -1).

First you can define the Point tuple to simplify coding:

import collections

Point = collections.namedtuple('Point', 'x y')

Then, you can use this named tuple to construct all possible directions:

import itertools

directions = [
    Point(*coord) for coord in itertools.product([-1, 0, 1], [-1, 0, 1])
]
directions.remove(Point(0, 0))

You get:

[Point(x=-1, y=-1), Point(x=-1, y=0), Point(x=-1, y=1), Point(x=0, y=-1), Point(x=0, y=1), Point(x=1, y=-1), Point(x=1, y=0), Point(x=1, y=1)]

For each direction, you can search for the possible moves. But before you need to calculate the next position of a token in a given direction. You can use a simple generator for that:

def next_pos(p, d):
    p0 = p
    while 0 <= p.x < 9 and 0 <= p.y < 9:
        if p != p0:
            yield Point(p.x, p.y)
        p = Point(p.x + d.x, p.y + d.y)

Then you can loop as follow:

# coordinates of the position of the chip
chip = Point(2, 4)

for direction in directions:
    print('In the direction: {0}'.format(direction))
    for pos in next_pos(chip, direction):
        ...

Here, pos is a Point instance that you can use to check if a position is valid…

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.