-2

I'm new to c++ programming and I'm trying to make a chessgame in which I have classes that represent chesspieces on a chessboard. I've made instances of chesstiles on the chessboard that contain instances of chesspieces. If I shift one chessPiece from one chesstile to another one is this good practice? or should I refrain from turning pointers into nullprt?

std::map<TLocation, ChessTile*> ChessTiles;

bool ChessBoard::MoveFromTo(TArray<FVector2D> moves, int32 fromX, int32 fromY, int32 toX, int32 toY)
{
    if (moves.Constains(FVector2D(toX,toY)))
    {
        ChessTile* chessTileFrom = ChessTiles.find({ fromX, fromY })->second;
        ChessPiece* chessPieceFrom = chessTileFrom->chessPiece;
        ChessTile* chessTileTo = ChessTiles.find({ toX, toY })->second;

        chessTileTo->chessPiece = chessPieceFrom;
        chessTileFrom->chessPiece = nullptr;

                return true;
    }
    return false;
}

I'm expecting this to make sure that the chesspiece wont be recognized by multiple tiles. As far as I know this should work, I am not getting any errors. But I'm just not sure if this is good practice or if I should not do this. It's hard to find it anywhere but that might be because I'm not at that level of coding yet.

Thanks in advance!

Defecter
  • 1
  • 2
  • It's highly recommended that you "move" large objects where possible. There is no shame in using `nullptr` to make an empty location, but you need to do it carefully. Since you are learning and don't want to have to deal with the kinds of errors you get from tripping over a null, consider having a generic blank space tile you can use to represent empty locations. – user4581301 Aug 29 '23 at 20:26
  • Side note: Sit down and think how you're managing the tiles through. Draw it out on a piece of paper. You may find that the tiles don't need to be anything more than a grid, a 2D array that either has a pointer to a piece or the empty place-holder discussed above. In general, the stupider you can be when writing code, the better off you'll be when it comes to sorting out the bugs. Stupid code tends to have fewer and simpler bugs. – user4581301 Aug 29 '23 at 20:30
  • Thanks for the advice. That is something I could indeed do. I actually have a 'hasChesspiece' boolean in the ChessTile class, I suppose I could also just ask for that. Even though the tile might actually contain something it could just block it from using it, that might also work. – Defecter Aug 29 '23 at 20:31
  • I was actually just thinking about that... the tiles right now the only thing the chesstiles do is spawn a piece based on what their vector position on the board is. But that might also be something the chessBoard could do. – Defecter Aug 29 '23 at 20:34
  • `TArray moves` -- This should be passed by const reference, not by value. – PaulMcKenzie Aug 29 '23 at 20:39
  • can't all of the variables be const references in that case? since I'm not changing any of the values? – Defecter Aug 29 '23 at 20:43
  • It is a direction worth exploring. You don't want to make the chessboard class too complicated with too many responsibilities, but you also don't want to have too many different moving parts to manage. You're always making trade offs in programming. – user4581301 Aug 29 '23 at 20:43
  • My rule of thumb is to start with the most restrictive ,`const` in this case, and move to less restrictive only when forced to. The compiler can catch extra bugs for you. – user4581301 Aug 29 '23 at 20:45
  • @Defecter Then at least you should pass by reference. Right now, you are incurring an unnecessary full copy of the `TArray`. – PaulMcKenzie Aug 29 '23 at 20:52
  • I’m voting to close this question because working code that you want feedback for belongs on [Code Review Stack Exchange](https://codereview.stackexchange.com/). StackOverflow is generally for when you have a specific problem that needs fixing. – Jan Schultke Aug 29 '23 at 20:53
  • @JanSchultke While this may be on-topic on CR, in the future, please don't use the existence of the Code Review site as a reason to close a question. Evaluate the request and use a reason like *Needs more focus* (as I have done here), *primarily opinion-based*, etc. Then you can mention to the OP that it can be posted on Code Review if it is [on-topic](https://codereview.stackexchange.com/help/on-topic). Please see [_Does being on-topic at another Stack Exchange site automatically make a question off-topic for Stack Overflow?_](https://meta.stackoverflow.com/q/287400/1575353) – Sᴀᴍ Onᴇᴌᴀ Aug 29 '23 at 20:56
  • Thanks for all the advice guys. sorry if I made an incorrect post. This is also my first time using stackoverflow. – Defecter Aug 29 '23 at 21:04
  • This question is at least border line on SO, because it asks about opinions and does not contain a [mre]. No surprise that you got (interesting) comments instead of true answers. You could get more in depth advices on [CodeReview](https://codereview.stackexchange.com/), but beware: the rules on CR require a *full* working code. That means that as currently asked, the question would be rejected there. – Serge Ballesta Aug 29 '23 at 21:23

0 Answers0