Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a piece dictionary to get_valid_moves(); improved readability. #15

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ElizabethGraham
Copy link

Added a piece dictionary to get_valid_moves() to reduce if statement usage. Broke up the queen, bishop, and rook move functions to make it play nicer with a dictionary call and improve readability -- this technically added more code, but the get_(piece)_move functions should be adjusted anyway.

…usage. Broke up the queen, bishop, and rook move functions to make it play nicer with a dictionary call -- this technically added more code, but the get_(piece)_move functions should be adjusted anyway.
@Brikaa
Copy link
Contributor

Brikaa commented Aug 24, 2020

@ElizabethGraham, could you elaborate more on why you want to split up the qbr_moves() function?

@ElizabethGraham
Copy link
Author

ElizabethGraham commented Aug 24, 2020

@ElizabethGraham, could you elaborate more on why you want to split up the qbr_moves() function?

Well my main reasoning is that, to me, the script is looking a little over if'd at the moment, but also a good thing to focus on would be objectifying and simplifying the script overall.

My main goal yesterday was to reduce if's first, so I implemented the dictionary call. Issue was the main get_(pawn/rook)_moves() took only r + c as arguments while get_qbr_moves() took the piece argument too. get_qbr_moves() was actually the most correct function there though and the goal should be to make one big function operate largely like it did, but in it's own pieces.py script. The way it's set up now, we can work towards only having one get_piece_moves function instead of multiple -- ultimately reducing the length of the program and making it far more readable while keeping a more branchless programming approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants