Rook movement function not finding interruptions

Question

I am trying to write a function that checks if the black king is in check by the white rook. The problem occurs when I try to search if there are any pieces in between the rook and the king.

    void rook_white(piece A[])                                      // does WR check BK
{
    cout << "Found White Rook ";
    int k_x;                                                  // BK'S x coordinate
    int k_y;                                                  // BK's y coordinate
    bool check = false;
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x-x][m_y] == 'k')                 // Moving Left
            {
                k_x=m_x;
                k_y=m_y;
                goto al_1;
            }
        }
    }
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x+x][m_y] == 'k')                 // Moving Right
            {
                k_x=m_x;
                k_y=m_y;
                goto al_2;
            }
        }
    }
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x][m_y-y] == 'k')                 // Moving Up
            {
                k_x=m_x;
                k_y=m_y;
                goto al_3;
            }
        }
    }
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x][m_y+y] == 'k')                 // Moving Down
            {
                k_x=m_x;
                k_y=m_y;
                goto al_4;
            }
        }
    }
al_1:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x-x][m_y] == '*')                   // Checking left
            {
                goto loop_exit;
            }
        }
    }
al_2:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x+x][m_y] == '*')                    // Checking Right
            {
                goto loop_exit;
            }
        }
    }
al_3:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x][m_y-y] == '*')                    // Checking Up
            {
                goto loop_exit;
            }
        }
    }
al_4:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x][m_y+y] == '*')                    // Checking Down
            {
                goto loop_exit;
            }
        }
    }
loop_exit:
    check = false;
    if (check == true)
    {
        ofstream fr(FVR,ios::app);
        fr << "Black is in check by rook " << t_i << endl;
    }
}

The function searches where is the king on the x and y axis. It then goes to a specific algorithm (al_1, al_2, ...) in which it searches if there is anything in between the king and the rook.

The input file is something along the lines of this:

********
***k****
********
***q****
********
********
***R****
********

this shouldn't output nothing, while

********
***k****
********
********
********
********
***R****
********

should output "Black is in check by rook ". (t_i being the number of the board it is being examined on)

A[t_i] is a struct array, the struct pieces consists of char field[8][8].

I use a struct because there is an unlimited number of boards I have to examine.

k_x and k_y are the king coordinates. m_x and m_y are global variables that are transferred from the search function in which a piece is found

here is the program itself with the int main()

#include <iostream>
#include <fstream>                  // chess
#include <cstdlib>
using namespace std;                                                       // PROGRAM CHECKS FOR CHESS CHEKS
const char FVD[]="5_input.txt";                                 // **************************************************
const char FVR[]="5_output.txt";                                // P - white pawn   (WP)      p - black pawn     (BP)
//---                                                           // R - white rook   (WR)      r - black rook     (BR)
//---                                                           // B - white bishop (WB)      b - black bishop   (BB)
int m_i=0;          // max i                                    // N - white knight (WN)      n - black knight   (BN)
int m_x=0;          //                                          // Q - white queen  (WQ)      q - black queen    (BQ)
int m_y=0;          //                                          // K - white king   (WK)      k - black king     (BK)
int t_i=0;          //                                          // **************************************************
struct piece
{
    char field[8][8];
};
void read(piece A[])
{
    ifstream fd(FVD);
    int i=0;
    m_i=0;
    while(!fd.eof())
    {
        for(int x=0; x<8; x++)
        {
            for(int y=0; y<8; y++)
            {
                fd >> A[i].field[x][y];
            }
        }
        fd.ignore();
        i++;
        m_i=i;
    }
    fd.close();
}
///  ----------------------------------------------BLACK KING IS IN CHECK--------------------------------------------------------------
void rook_white(piece A[])                                      // does WR check BK
{
    cout << "Found White Rook ";
    int k_x;                                                  // BK'S x coordinate
    int k_y;                                                  // BK's y coordinate
    bool check = false;
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x-x][m_y] == 'k')                 // Moving Left
            {
                k_x=m_x;
                k_y=m_y;
                goto al_1;
            }
        }
    }
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x+x][m_y] == 'k')                 // Moving Right
            {
                k_x=m_x;
                k_y=m_y;
                goto al_2;
            }
        }
    }
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x][m_y-y] == 'k')                 // Moving Up
            {
                k_x=m_x;
                k_y=m_y;
                goto al_3;
            }
        }
    }
    for(int x=0; x<8; x++)
    {
        for(int y=0; y<8; y++)
        {
            if(A[t_i].field[m_x][m_y+y] == 'k')                 // Moving Down
            {
                k_x=m_x;
                k_y=m_y;
                goto al_4;
            }
        }
    }
al_1:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x-x][m_y] == '*')                   // Checking left
            {
                goto loop_exit;
            }
        }
    }
al_2:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x+x][m_y] == '*')                    // Checking Right
            {
                goto loop_exit;
            }
        }
    }
al_3:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x][m_y-y] == '*')                    // Checking Up
            {
                goto loop_exit;
            }
        }
    }
al_4:
    for(int x=0; x<k_x; x++)
    {
        for(int y=0; y<k_y; y++)
        {
            if(!A[t_i].field[m_x][m_y+y] == '*')                    // Checking Down
            {
                goto loop_exit;
            }
        }
    }
loop_exit:
    check = false;
    if (check == true)
    {
        ofstream fr(FVR,ios::app);
        fr << "Black is in check by rook " << t_i << endl;
    }
}
///-----------------------------------------SEARCHING FOR THE CHECKS // CALLING FUNCTIONS --------------------------------------------
void search(piece A[])                                               // searches for piece and calls a function related to it
{
    for(int i=0; i<m_i-1; i++)
    {
        for(int x=0; x<8; x++)
        {
            for(int y=0; y<8; y++)
            {
        if (A[i].field[x][y]=='R')
                {
                    t_i=i;
                    m_x=x;
                    m_y=y;
                    rook_white(A);
                }

            }
        }
    }
}
int main()
{
    piece A[10];
    remove(FVR);            // clears the output before inputting new data because of ios::app
    read(A);
    search(A);
    ofstream fr(FVR);
    fr << "Done";
    return 0;
}

Answer 1

I'll do what I don't usually do -- I'll give you a full round-trip on this one (because I feel like it).

---

You started with a main() that should read an arbitrary number of fields (but actually crashes on the 11th), and a search() function that is intended to eventually iterate through all possible chess pieces... but your program fails on the check for the very first piece, the white rook.

You also started with C, and then introduced homeopathic doses of C++, actually making things more difficult than necessary.

So let's trim this down. First, we'll get the "minimal" into your example code.

---

Let's use <vector> instead of a two-dimensional array. Vectors are one of the most useful things in C++, and there is really next to no reason for using C-style arrays anymore (except perhaps for C API compatibility).

#include <vector>
#include <iostream>

typedef std::vector< std::vector< char > > Field;

Field is now an alias for a vector of vectors of chars. That's basically the same as a two-dimensional array (including the [][] addressing), but it has some advantages as you will soon see.

---

Your rook_white() (as you designed it) needs one field, and the rook's X and Y coordinates. Allow me to tweak your function prototype a bit -- I like more expressive identifier names.

void rook_white( Field const & field, unsigned rook_x, unsigned rook_y );

Search-and-delete A[t_i]., we only work on that one field for now. Search-and-replace m_x with rook_x and m_y with rook_y. Not too hard. Also replace:

        ofstream fr(FVR,ios::app);
        fr << "Black is in check by rook " << t_i << endl;

With:

        std::cout << "Black is in check by rook" endl;

For now we don't bother with file I/O.

---

Now, we need to set up a field, but we do not need to read it from file for now. You can expand your code once you got the actual check working.

int main()
{
    Field field( 8, std::vector< char >( 8, '*' ) );

std::vector< char >( 8, '*' ) creates a temporary vector of 8 '8' chars. Field field( 8, /*...*/ ); creates a Field consisting of 8 copies of that temporary vector.

Now let's place your pieces.

    unsigned rook_x = 3;
    unsigned rook_y = 6;
    field[rook_x][rook_y] = 'R';

    unsigned king_x = 3;
    unsigned king_y = 1;
    field[king_x][king_y] = 'k';

At this point I realized your example code got "X" and "Y" coordinates mixed up in search() (reporting the rook at X = 6, Y = 3), but nevermind. That's not the source of your problems.

We now have field and coordinates to call your function.

    rook_white( field, rook_x, rook_y );
    return 0;
}

This way of writing a main() that doesn't really reflect what your final application should do, but rather sets up a test for a specific functionality, is called a test driver. We just shaved over 50 lines of unnecessary code from your example, eliminating all sorts of unrelated potential problems.

---

Now, let's have a look at rook_white(), shall we?

---

Since we now have a vector< vector< char > > instead of a "dumb" array, we could do something nifty: Replace [] access with .at(). The reason? If the index to [] is out-of-bounds, it might or might not crash the program. If the index to .at() is out-of-bounds, it will throw an exception.

So,, while .at() is (a bit) slower and usually not used in production code, I recommend it for beginners because it does not allow errors to "hide".

At this point you should be arcing an eyebrow and think to yourself, "why is he suggesting this?". Then you should look at your loops, and...

for(int x=0; x<8; x++)
{
    for(int y=0; y<8; y++)
    {
        if(field[rook_x-x][rook_y] == 'k')
        {
            k_x=rook_x;
            k_y=rook_y;
            goto al_1;
        }
    }
}

Yes, exactly. You have out of bounds accesses to your field.

rook_x / rook_y is somewhere in the midst of the field, but you insist on accessing anything up to [rook_x - 7][rook_y] if you cannot find the king. That was a negative index in your original code. Since I changed the coordinates to unsigned (which instead overflows and becomes really large) you'll get a segfault crash instead (if you're lucky). That was actually unintentional; I just declare things that cannot be negative unsigned by habbit.

But that's why I suggested using vector<>'s .at() method while you're still learning: Fail as early and as loud as possible. An exception is better than undefined behaviour.

---

Besides, you do (in all those loops) always loop over x and y, but only use one of the two variables inside the loop. That's a lot of wasted clock cycles. That this doesn't break your logik is mere chance...

At this point you probably want to completely re-work your code. But wait, there is more.

---

If you "move left" in your first loop, and find the king there, you goto al_1. There you loop (again using only one of the two loop counters) to check for intervening pieces.

The first loop is x == 0, checking [rook_x - 0][rook_y]... well guess what, you find the white rook to be intervening there, since there is no '*' in that field, so you jump to loop_exit...

---

And even if you wouldn't have made that mistake, you would come out of that loop and enter al_2, checking all remaining directions for the rook as well...

---

And even if all that wouldn't happen, no matter what happens, you eventually run into this:

loop_exit:
    check = false;
    if (check == true)
    {
        std::cout << "Black is in check by rook \n";
    }

Well, that check == true is never going to happen, right?

---

At this point I quote one of your comments...

...I just don't understand [switch statements] and couldn't really wrap my head around on how to write them as switch.

My suggestion? I fully understand why you want to write "something real" as fast as possible. Textbooks are boring. But you really should spend some more time "wrapping your head around" basic concepts. C and C++ are languages that really don't work that well with "trial & error" approaches to learning them. There is simply too much that can, and will, go wrong.

We have a list of recommended textbooks, if the one you have isn't to your tastes.

And if you try too large things (like a chess program) before you really got the basics right, the answer to any questions you might have will end up being quite a bit longer than comfortable, both to write (if someone feels like it) and for you to digest.

---

And please:

Don't use goto unless you absolutely, positively, know what you're doing.