5
\$\begingroup\$

This is my first little python project. I want to get to a point where i can solve chess puzzles like this one from puzzling.stackexchange. I want to be able to change the board size to something like 15*15 or 3*7 and still all rules to apply.

So far, you can just run the file and it gives all possible moves for a knight on a random starting position. You can also put it a given position in string form like "A5" for the variable "startKnight".

What can I do better? The letter number swap function to swap between the chess and matrix connotation looks so big and ugly... The knightmoves function looks good to me, but maybe you can tell me what to improve :)

from random import randint

def letterNumberSwap(x):
    # change letters to numbers and vice versa
    if x == "A":
        return 1
    elif x == "B":
        return 2
    elif x == "C":
        return 3
    elif x == "D":
        return 4
    elif x == "E":
        return 5
    elif x == "F":
        return 6
    elif x == "G":
        return 7
    elif x == "H":
        return 8
    elif x == 1:
        return "A"
    elif x == 2:
        return "B"
    elif x == 3:
        return "C"
    elif x == 4:
        return "D"
    elif x == 5:
        return "E"
    elif x == 6:
        return "F"
    elif x == 7:
        return "G"
    elif x == 8:
        return "H"
    else:
        print("Something is wrong.")

moveList = [] # list of moves in matrix connotation
possibleMoves = [] # list of moves in chess connotation

def knightMoves(position):
    # calculate all knight moves from position
    column, row = list(position.strip().upper())
    column = letterNumberSwap(column)
    c,r = int(column), int(row)
    if (0 < (c - 2) <= 8):
        if (0 < (r - 1) <= 8):
            moveList.append([c - 2, r - 1])
        if (0 < (r + 1) <= 8):
            moveList.append([c - 2, r + 1])
    if (0 < (c - 1) <= 8):
        if (0 < (r - 2) <= 8):
            moveList.append([c - 1, r - 2])
        if (0 < (r + 2) <= 8):
            moveList.append([c - 1, r + 2])
    if (0 < (c + 1) <= 8):
        if (0 < (r - 2) <= 8):
            moveList.append([c + 1, r - 2])
        if (0 < (r + 2) <= 8):
            moveList.append([c + 1, r + 2])
    if (0 < (c + 2) <= 8):
        if (0 < (r - 1) <= 8):
            moveList.append([c + 2, r - 1])
        if (0 < (r + 1) <= 8):
            moveList.append([c + 2, r + 1])
    for entry in moveList:
        # back to chess connotation
        possibleMoves.append(letterNumberSwap(entry[0])+str(entry[1]))

randomStart = str(letterNumberSwap(randint(1,8)))+str(randint(1,8))
startKnight = randomStart
knightMoves(startKnight)
print("A Knight on " + startKnight + " can go to:")
for entry in possibleMoves:
    print(entry,end = ' ')
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

Review

There are a few things that can be improved upon here,

def letterNumberSwap(x):

  • is reinventing the wheel of str.index()
  • and str[index]

randomStart = str(letterNumberSwap(randint(1,8)))+str(randint(1,8))

  • random.choice() might be better choice :)

    It's just implementation details. randint delegates to randrange, so it has another layer of function call overhead, and randrange goes through a lot of argument checking and other crud. In contrast, choice is a really simple one-liner. source

moveList = [] # list of moves in matrix connotation
possibleMoves = [] # list of moves in chess connotation
  • Stick to the scope.

    Do not make these globals,.. either return or yield them from the function

if (0 < (c - 2) <= 8):
    if (0 < (r - 1) <= 8):
        moveList.append([c - 2, r - 1])
    if (0 < (r + 1) <= 8):
        moveList.append([c - 2, r + 1])
  • Stick to DRY, you can avoid the repetition by saving all possible directions in a list

  • Don't do string + string but use .format() or use the fancy f"{string}" if you use python 3.6+

  • in range(x) is alot faster in python3 instead of x <= !! and x >= !! @Graipher

  • Naming

    In chess a row is called a rank, and a collumn is called a file

  • Avoid magic numbers

    Make the WIDTH and HEIGHT constant so that they can easily be changed

Alternative

from string import ascii_uppercase
from random import choice

BOARD_HEIGHT = 5
BOARD_WIDTH = 13
CHESS_FILES = ascii_uppercase[:BOARD_WIDTH]
CHESS_RANKS = range(1, BOARD_HEIGHT+1)

def knight_moves(file, rank):
    position = (CHESS_FILES.index(file), rank)
    directions = ((1, 2), (1, -2), (-1, 2), (-1, -2), (2, 1), (2, -1), (-2, 1), (-2, -1))

    for direction in directions:
        file, rank = (d + p for d, p in zip(direction, position))
        if  file in range(BOARD_WIDTH) and rank in CHESS_RANKS:
            yield f"{CHESS_FILES[file]}{rank}"

if __name__ == '__main__':
    file = choice(CHESS_FILES)
    rank = choice(CHESS_RANKS)

    print(f"With BOARD({BOARD_WIDTH}, {BOARD_HEIGHT})")
    print(f"A knight on {file}{rank} can go to:")
    print(", ".join(knight_moves(file, rank)))
\$\endgroup\$
4
  • \$\begingroup\$ Thank you for your answer. I wanted to be able to have rectangular boardsizes as well, but I think I can get there from here. Why is random.choice better? \$\endgroup\$
    – Tweakimp
    Dec 12, 2017 at 15:24
  • \$\begingroup\$ It is shorter for one, CHESS_FILES[randInt(CHESS_FILES)] is the same as choice(CHESS_FILES) \$\endgroup\$
    – Ludisposed
    Dec 12, 2017 at 15:27
  • \$\begingroup\$ There is something wrong with the allowed directions: I got:" With BOARD(4, 4) A knight on A2 can go to: B0, C3, C1 " C0 is not a valid move. \$\endgroup\$
    – Tweakimp
    Dec 12, 2017 at 19:44
  • \$\begingroup\$ @Tweakimp, correct, I had a leftover typo\ in there... fixed it with rank in CHESS_RANKS \$\endgroup\$
    – Ludisposed
    Dec 13, 2017 at 8:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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