Crazy chess: board representation

First a note: I spent a bunch of time writing code without blogging. The first issue is pretty far along. This first code analysis entry though is going to be just about the board representation.

At this point it’s tempting to anticipate the future and to make something that addresses concerns we expect to come up. This is best avoided though, and so at this stage of development the requirements on this board are quite trivial.

First the unit tests. Really all we need to do is make sure we can build a new board and then fill it with pieces:


BOOST_AUTO_TEST_CASE(empty_board)
{
    auto board = crazychess::board{};

    for (auto const& space : board)
    {
        BOOST_CHECK(space == crazychess::pieces::empty);
    }
}

BOOST_AUTO_TEST_CASE(occupied_spaces)
{
    auto board = crazychess::board{};

    board[0] = crazychess::pieces::white_pawn;
    board[1] = crazychess::pieces::black_king;
    board[2] = crazychess::pieces::white_rook;

    BOOST_CHECK(board[0] == crazychess::pieces::white_pawn);
    BOOST_CHECK(board[1] == crazychess::pieces::black_king);
    BOOST_CHECK(board[2] == crazychess::pieces::white_rook);

    std::for_each( std::begin(board)+3, std::end(board)
                 , [](crazychess::piece p) { BOOST_CHECK(p == crazychess::pieces::empty); });
}

The first test just checks that the default constructor creates an empty board. I don’t know yet if this is the right thing to do, but if not I can always delete this test–this is something that is important to get used to. As soon as a test becomes obsolete or becomes a maintainence burden you should look for ways to decrease that burden or just delete the test entirely–maybe start over, maybe not.

The second ensures I can modify the board to have non-empty squares. I don’t do an exhaustive check, that’s a bit overkill, but I do want to make sure that the basic interface works.

The code that passes these tests is quite trivial:

enum struct piece
{
    empty = 0
  , white_pawn
  , white_rook
  , white_knight
  , white_bishop
  , white_queen
  , white_king
  , black_pawn
  , black_rook
  , black_knight
  , black_bishop
  , black_queen
  , black_king
};
using pieces = piece;

using board = std::array<piece, 64>;

First a note what this code does because it may look a bit funny to people used to C++03 or are learning at college where they’re not instructing in the new language yet.

The first bit defines a new enumeration type. Unlike in previous C++, this one defines a truly unique type and not just a bunch of integer values. I couldn’t pass a black_rook as a color for example. This introduces some bits of inconvenience as you’ll see later in some position work that uses bitmasks but it also introduces clarity and safety to the code–I value the latter more than convenience. The explicit use of 0 to assign empty is superfluous but it also adds clarity–one could go either way on that since a C++ developer should know that the first element in an enum will be 0 unless otherwise stated, but I just assume I don’t need to judge and add clarification when I think of it and it has no extra cost such as in this case.

Next I create an alias for piece so that plural sense can be used when it makes sense. This simply increases the vocabulary options in the language we are creating to speak about the constructs in the code. Code should be expressive to humans and so we want to enable humans to communicate well. As you create a new program you’re also creating a language that communicates the construction of that program and will impose on your ability to communicate as you go.

Next I just create an alias for an array that is called “board”. This means that the board indentifier throughout the code (when that identifier isn’t overloaded in a more limited scope) will have the same syntactic meaning as an array, though semantically, to humans, the name gives it slightly more meaning. We have to watch out here though because in this case “board” and “array<piece,64>” are exactly the same types–you can interchange them implicitly and so if you are using the same basic type to represent a different concept you might have troubles.

An argument here can certainly be made that this code represents a case of “primitive obsession”. In fact we can already get a sense that we’re on a bad track by looking at some unit test code I created:

std::string quick_board_string(crazychess::board const& board)
{
    auto result = std::string(64, ' ');

    std::transform( std::begin(board), std::end(board)
                  , std::begin(result)
                  , [](crazychess::piece p)
                    {
                        constexpr auto pc = " PRNBQKprnbqk";
                        return pc[static_cast<int>(p)];
                    } );

    return result;
}

This seems like pretty inoculous code, and to some degree it is, but it represents a potential to spread logic around the system rather than to encapsulate it locally. If I made a fen_generator(board const&) function it would necessarily contain a lot of the same logic. This is quite undesirable and is one of the first things that will cause your project to become unmaintainable pretty darn quickly. When something about the board class changes for example, all of these functions that implement this same logic will have to be changed. Some may get lost in the shuffle if the type system doesn’t catch it, and since I used a rather primitive type here that becomes more likely.

Here also though is a great temptation to be too darn perfect. If we spend all our time analyzing every potential problem and dealing with it we’ll stagnate and get nothing done. There’s a bit of a balance to hunt down here and there’s not really any clear way of deciding who’s being too sloppy and who’s being too analytical. Except for extremes on either end, which are both quite detrimental to your success as a professional developer, there’s a lot of wiggle room and subjective likes that come into play.

One way that you can handle this uncertainty, and its the proceedure recommended in agile development, is to just write whatever code you need to pass the tests you have. Then later, previous to making your potential commit changeset, you look at what you’ve written and look for smells that already exist and can be cleaned out. If you have a bunch of copy-pasta (code that’s been copied to numerous locations and then perhaps modified slightly) you now look for ways to consolidate shared logic. This has the advantage of getting you to a point where you have something working pretty rapidly so that if your employer or customer cuts you off–like they need the feature NOW–you can toss it out the door.

This later action is a decision that is sometimes made, perhaps by you, perhaps not–maybe you’re being forced to do it against your objections. The choice to release code that is not quite the cleanest it could be but gets the job done is known as “Technical Debt”. This is a debt that you absorb by deciding to make your later self more miserable in some way. Either you’ll have to go back and pay the debt by fixing the unclean code, preferably fairly soon, or you’ll have to constantly pay it as you move forward. When too much technical debt develops your project will collapse, usually long before management actually decides to discontinue it or pay the debt.

Some people or employers consider bugs technical debt. They might let bugs build up and then do a “technical debt payment rush” to fix as many as they can and get the defect count down. I do not agree that they’re the same thing. Technical debt is code that actually does what it is meant to, it just represents a continued cost in time and mental work needed to maintain the product. Bugs are actually defects in the program that will impact some customers. It is true that we often put off fixing them because they only affect a few customers or the fixing of them is too great (usually because of a decision to take on actual technical debt earlier in the process), but technical debt is debt that a company or team takes on of its own…its only affect on the customer is in feature request or issue turnover.

At this point I’m taking on this technical debt. As I get closer to actually considering this task complete and submit a pull request (or code review if you’re not using github) I will look back and see if there’s debt I do not want to take on. I have this luxury since I’m in charge, usually you won’t.

As a general rule I disfavor primitive obsession and think it’s a huge mistake so I imagine I’ll find some practical excuse that requires I create real types rather than aliases. I certainly see forces that might come in the future that would require them, but at the same time the temptation to go off trying to anticipate the future is probably one of the strongest (at least for me) so I may instead force myself to stop where I am. I will let the future decide.

Much more than this has been checked in and you’re encouraged to look into it. This entry though represents the code as it was at this point in the history.

As I move forward you should feel free to, if you have a github account, comment on changesets. You can comment on any. Ask questions, suggest alternatives, etc… Probably the most important part of being a professional developer is collaboration. It can also be a pretty hard one to develop–many of us would prefer to instead be lone wolf developers. One great way to learn, besides teaching under guidance of peers, is to ask questions and critique others’ code. I encourage you therefor to comment on mine, especially as it pertains to this project. You can comment on any changeset, including this one.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s