Hot answers tagged

34

My comments are with respect to design of the game. I see responsibilities of entities are mixed up in many places Player should not initialize pieces. We can move the responsibility to board. Board and pieces doesn't need to know about players. Pieces should not handle move. Pieces can provide list of possible moves to reach the destination path but board ...


28

Without offering a deep code review (as I don't have a lot of specific Java knowledge), let's look at what a full "move" entails in chess: Player chooses piece to move. Piece makes legal move according to its own move rules. In addition to purely move-based rules, there's also capture logic, so a bishop cannot move from a1-h8 if there's a piece sitting on ...


19

Some quick shots a if condition written like if (booleanVariable==true) can be simplified to if (booleanVariable) you shouldn't have public variables like public boolean white; no constructor of Game, Board, Player and Piece should call super() because they are obviously not inheriting / extending any class. Some design quickshots a chessgame ...


19

There are a few traps you're getting yourself into a bit. By definition your white rook starts at a specific spot. What are you going to do for the other rook? Also you're using class inheritance White_Knight(chess_board) in a funny way. Please read this quick chapter. In short, your implying that your white knight IS a chess board. But your white ...


14

I spent some time last night reviewing this code. Here are some specific suggestions I can make: Move your game loop into an if __name__= "__main__": statement at the bottom of the code. Currently, if the module is imported, the person who imports it will immediately start playing chess, which isn't desirable (they may just want to know about the piece ...


14

This is not C++ Calling this C++ is disingenuous (claiming C++11 nope). This is a C program that just happens to use a few C++ objects (std::cout and std::cin) and the type std::string. This style is commonly derisively referred to as C with Classes. My problem here is that C++ is a style and the way you use the language is what differentiates it from C. ...


13

for (unsigned char it = 0; it <= 1; it++) { for (signed char xy = (it==0 ? piece_pos.x_pos:piece_pos.y_pos) + 1; xy <= MAX_RANGE; xy++) { co_ord temp (it == 0 ? xy:piece_pos.x_pos,it == 0 ? piece_pos.y_pos:xy); if (isAvailableSquare(temp,piece_type)) tempmoves.append(temp); if (!brd.getSquare(temp)....


12

This is a lot of work, and I don't have a lot of time, but I thought I'd throw in my two cents. So, here's what I've got for you: The Good Your models are nicely formed. More than a data store, they actually do stuff. This is good practise. You've compartmentalised the code into objects that are easy to read and follow. Good job. The Bad You're ...


12

numpy In this case there is no need to use. For an 8 by 8 board, filled with strings, there is no advantage to using it, apart from the possibility to index row and column at the same time enums You have a few properties wich would be best presented as an enum. The color and the type of the piece import enum class Color(enum.Enum): WHITE = 0 ...


11

You should separate the modelling of the game, board and pieces from the display components. (google: model-view-controller, separation of concerns). You can create a Game class that models the game and the game-logic You can create a Board class that models the internal state of the chess-board. You can create a Piece class and subclasses for each Piece, e....


11

I think it definitely makes sense to use an object hierarchy here. What's the difference between a black and a white pawn? Its colour! (which will affect which way it can move relative to the board). All pieces must have exactly one colour, and we have two to chose from: public enum PieceColour { White = 0, Black = 1 } As we've already reasoned, ...


11

Instead of using a boolean to flag if something is something (and thus implying that it isn't the other thing), consider an enum; in this case that doesn't buy you anything in terms of flexibility (since it's not as if there will ever be, say, red or purple or whatnot) but it will make your code much more clear. On the game, instead of setting black and ...


11

First observation: you should probably be thinking about separating the model of the game from the presentation of the game. For example: public interface Piece { public List<Position> posMoves(ChessBoard board, Position pos); public Icon image(); posMoves is about the game in the abstract - that's clearly a representation of the rules of ...


10

The thing that looked the slowest was int mask = 0; for (int i = 0; i < depth; i++) { int queen = candidates[queens[i]]; mask |= queen; // previous queens cover this column.... mask |= queen >>> (depth - i); // previous queens cover this // diagonal to the right. mask |= queen << (depth - ...


10

Lets start by naming: It should be clear that the frame doesn't have the name of the project, so instead of the class ChessProject you can call this class ChessFrame instead. It's all about separation of concerns, you should have seperate class that does the Chess logic and one that does the UI and use composition to use its features class ChessBoard{ .....


10

Does it smell? Regarding the general design: You are mixing the view and the model, which makes your code inflexible, and makes it harder to find the relevant pieces of code. the loading of the image icon seems to always be the same, I would create an abstract piece class and move it there. same goes for the getters. A boolean for the color is not optimal, ...


10

This code is improved and I see that you've added the command shoot. I think there are still some things you could do to further improve the code. Enforce the rules It's a bit disappointing to be able to win in two moves: shoot 9 16 8 1 shoot 9 16 9 1 This shouldn't be possible because the dragon should not be able to shoot across the entire board. The ...


10

I feel @spyr03's answer is great (+1 it), but I would like to extend it a but further in a number of ways. Algorithm The suggestion that you can validate the move by checking one axis moves 2, and the other moves 1, is an interesting, but not ambitious enough solution. A knight moves in a right-angled pattern, with 2 steps on one side, and 1 on the other. ...


10

return type char should only be user for characters, as it's name says. unsigned char for characters or for aliasing other types. For everything else, use anything else. If you want (u)int8_t, use <stdint.h>, and if you don't have that header, feel free to write your own typedef (enclosed in an #if). But for error codes, the best is to use good ...


9

First, you can make your empty board much faster. For a start, you can create a list of 8 '.'s with the multiplication operator. board = ['.'] * 8 And you can make it have the 2 axes with a thing called a list comprehension. It's basically a shorthand for generating a list with a for loop.: board = [['.']*8 for _ in range(8)] Using _ in this loop is just ...


9

Switch your code up We can make this much more readable with a switch statement. switch(piece_abs_val) { case 1: if(piece_val != -1){ //w_pawn if(piece_direction == 4 || piece_direction == 6){ is_attacked = true; } } else { //b_pawn if(piece_direction == 2 || piece_direction == 8){ ...


9

using System.Text; using System.Threading.Tasks; What are these doing there? (The obvious answer would be that you used the default template in VS and didn't bother to clean up, but I hope that you have a better explanation than that). static void Main(string[] args) { ChessGame game = new ChessGame(); } When I compile ...


8

One thought ... Instead of having all those else blocks that only have a return statement, switch your if statements around, so that the if block only has a return statement, and then there doesn't need to be an else block for it. E.g., instead of ... if 1<=X1<=8: original_contents_of_if_block else: return -1 write ... if not(1<=X1<=8)...


8

What an interesting challenge. Learning Java8 is on my list, so here's some suggestions I have, but bear in mind that I am learning too.... First up, lets use functions for the functions we have. Starting with the safe function: private static boolean safe(Pos p, Pos q) { return p.x != q.x && p.y != q.y && abs(p.x - q.x) != abs(p.y - q....


8

I liked reading your code and thinking about it. While trying to find out what to improve I almost started doing all the refactoring that came to mind. But I stopped myself thinking it might be better if I describe what I think could be improved and you try factoring those ideas into your code. What I think is very good style is that you try to avoid most ...


8

Thinking about it, I bet a lot of it could be cut right down if the function actually didn't care about moves going off-board, like you say, and that was tidied up later. Hmm. Probably people who have written chess games before have much better ideas, but here are my thoughts. (Yeah, I've been replacing the nested lists of lists with tuples). Pawns The ...


8

By measuring with JMH I found your decoder being much slower than the encoder. Contrary to your conclusion that the bottleneck is array allocation, I find that the difference between one static array and a new array each time has secondary effect. Compare these results (single static array): MeasureEncoding.decodeBitfield avgt 5 5.896 ± 0.091 ns/op ...


8

Use Standards Use standard notations and representations whenever possible: see Algebraic Notation for example. Avoid Global Variables Board.CHESS_BOARD is a global variable. Global variables are bad. So instead of a global variable, pass the board as an argument to the method. It is very possible that a program may need to consider multiple chessboard ...


8

Style The overall style is good and mostly follows the official style guide. Since you have a lot of code and the scope will likely expand even more, I would highly recommend using an automatic style checking tool like pylint, flake8, or the like. There are a lot of Python IDEs which integrate nicely with those style checkers (Visual Studio Code, PyCharm, ...


7

Using enums instead of constants is a good fit if you deal with a familiy of something, like you do with Piece. As we don't know how the remaining code looks like, it is only a guessing what would be the best fit for your case at all. If your Piece'es also need some logic (properties or methods), you should go with what @Evorlor has commented. For ...


Only top voted, non community-wiki answers of a minimum length are eligible