C++

Forum

 
Perfecting Knight Movement in Chess

AmmmG 01 (51)
So this is what we have reached in this project, which finds the possible
moves for a knight.
What I am asking here is that could someone find a more simple way to
implement the set_if_valid? by basically having it in the possible_knight_moves?

and what does the void clear function does exactly?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
 
#include <iostream>
using namespace std;
const int n = 8 ;
using board_type = bool[n][n] ;

void clear( board_type& board ) { for( auto& row : board ) for( auto& v : row ) v = false ; }

void set_if_valid( board_type& board, int row, int column )
{ if( row >= 0 && row < n && column >= 0 && column < n ) board[row][column] = true ; }

void possible_knight_moves( board_type& board, int row, int column )
{
    clear(board) ;

    if( row >= 0 && row < n && column >= 0 && column < n)
    {
        for( const int row_delta : { -2, +2 } ) for( const int column_delta : { -1, +1 } )
        { set_if_valid( board, row + row_delta, column + column_delta ) ; }

        for( const int row_delta : { -1, +1 } ) for( const int column_delta : { -2, +2 } )
        { set_if_valid( board, row + row_delta, column + column_delta ) ; }
    }
    else cout<< "Wrong coordination" <<endl;
}

int main()
{
    board_type board {} ;

    int row = 0;
    int column = 0 ;
    
    cin >> row;
    cout << endl;
    cin >> column ;

    possible_knight_moves( board, row, column ) ;

    for( int row = 0 ; row < n ; ++row ) for( int column = 0 ; column < n ; ++column )
    { if( board[row][column] ) cout << row << ", " << column << '\n' ; }
}
Last edited on
TarikNeaj (2580)
So this is what we have reached in this project

You really haven't reached anything, @JLBorges wrote every line of code for you.


What I am asking here is that could someone find a more simple way to
implement the set_if_valid? by basically having it in the possible_knight_moves?

and what does the void clear function does exactly?


This is your job. He gave you a great reference to look at and learn from, not to copy. Learn from it and make your own version of it. Study it, try and understand what happens. Learn to debug. With debugging you can step through every line of code and see exactly what happens.

This is just my 2 cents.
Last edited on
AmmmG 01 (51)
you can go to my account and see everything anyway, so I wasn't claiming anything to begin with. We did a great job at it, but I decided to use his version and just play with it and learn from it.

But I really just wanted an explanation of the lines I referred to.
TarikNeaj (2580)
I didnt mean any offense by it or anything. But really, learn how to debug and save yourself a lot of trouble.

and what does the void clear function does exactly?

The clear function does exactly what it says in the name. It clears the board. It sets everything to false.
Last edited on
goosestuf (22)
Wouldn't learn too much from it, the code style is awful, you should never have nested for loops on a single line like that. It makes it so easy to miss and reduces readability. Along with unnecessary code duplication on lines 17/18 and 20/21.

Anyways looks like he just used a 2d array of bool type. Where true means the piece can move to that location. What clear does is set everything to false, meaning no moves can be made. They are then filled in with true to indicate that that location is a possible move. It isn't that complicated. set_if_valid is pretty simple, i mean it's a single if statement that only does one thing, that is set a value to true. Do you want to implement it a different way? Have to explain more.
cire (8284)
Wouldn't learn too much from it, the code style is awful, you should never have nested for loops on a single line like that. It makes it so easy to miss and reduces readability.

You're entitled to your opinion, of course, but I would be hard pressed to look at that function definition and "miss" the fact that it contains a nested loop.


Along with unnecessary code duplication on lines 17/18 and 20/21.

It often isn't worth removing code duplication (especially when it requires as much code as you're getting rid of) in one-off pieces put together off-the-cuff for students to learn from. Hardly worth a comment. Especially when the 'learning' clearly applies to understanding the logic as opposed to what style of coding to use.
AmmmG 01 (51)
Thanks guys.

But I am still wondering why would I need to use row_delta and column_delta?
What are they exactly to be honest?

I wish to find a different way to implement it, honestly!!
goosestuf (22)
cire wrote:
You're entitled to your opinion, of course, but I would be hard pressed to look at that function definition and "miss" the fact that it contains a nested loop.


Sure you would think so, but after a long days work or your tired or any other amount of factors. Having it all on one line just calls for disaster. You aren't also just writing for yourself you are writing for everyone else reading your code. Can you say the same for everyone else? You can't really dispute that using proper indention makes it much easier to understand what is going on.

1
2
3
4
5
6
7
8
9
10
11
12
13
 
for( const int row_delta : { -2, +2 } ) for( const int column_delta : { -1, +1 } )
{ set_if_valid( board, row + row_delta, column + column_delta ) ; }

// vs.

for( const int row_delta : { -2, +2 } )
{
    for( const int column_delta : { -1, +1 } )
    {
        set_if_valid( board, row + row_delta, column + column_delta );
    }
}



It often isn't worth removing code duplication (especially when it requires as much code as you're getting rid of) in one-off pieces put together off-the-cuff for students to learn from. Hardly worth a comment. Especially when the 'learning' clearly applies to understanding the logic as opposed to what style of coding to use.


It isn't about whether there is more or less code. It's about writing code that is less prone to error and if you have two pieces of code that are exactly the same then that just increases the risk of error when you have to make changes to it. If you are writing code that students learn from, can you really expect them to know what they are suppose to be learning from it? If you write bad code they are going to see that and write code that's just as bad as that cause that is what they learnt from. You can't nitpick when creating code that someone is going to learn from, cause there is no telling what they will learn from it. In that regard you shouldn't be lazy, otherwise you shouldn't be helping someone at all as you are just hurting them giving them your lazy code to learn from.
Last edited on
cire (8284)
Sure you would think so, but after a long days work or your tired or any other amount of factors. Having it all on one line just calls for disaster. You aren't also just writing for yourself you are writing for everyone else reading your code. Can you say the same for everyone else? You can't really dispute that using proper indention makes it much easier to understand what is going on

Personally, I find the first version more readable. No, I can't say the same for everyone else... and you obviously can't either.


It isn't about whether there is more or less code.

The parenthetical was an aside.


You can't nitpick when creating code that someone is going to learn from, cause there is no telling what they will learn from it.

Apparently, you can.


In that regard you shouldn't be lazy, otherwise you shouldn't be helping someone at all as you are just hurting them giving them your lazy code to learn from.

Not removing all code duplication from a one-off is not going to hurt anyone learning, and I would classify it as practical before I would lazy.
goosestuf (22)
Personally, I find the first version more readable. No, I can't say the same for everyone else... and you obviously can't either.


I can't say for you but i can for the most part say for everyone else as well. Cause you don't even need to read the code to be able to understand at least something about the code to know what it might be doing. That is the geometric structure of the code with the indentation. So if you can know something about the code without actually having to read the code, that would (by common sense, not bias trying to defend a friend cough cough) make it easier to understand than something where you would actually have to read the code to even remotely know anything about what is happening.

Apparently, you can.

Yes for the better, not writing horrid code people will learn from.


Not removing all code duplication from a one-off is not going to hurt anyone learning, and I would classify it as practical before I would lazy.

No it's definitely lazy.
booradley60 (896)
Oh, looky, an ongoing thread in the proper forum to debate stylistic dogma: http://www.cplusplus.com/forum/lounge/183595/

OP, you're basically saying you didn't understand the help that was offered to you. Why didn't you keep the thread going where JLBorges originally posted this snippet?

Here's the same code with a few layout updates and some comments:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
 
#include <iostream>
using namespace std;
const int n = 8 ;
using board_type = bool[n][n] ;

//The way we're going to solve the problem of describing which moves a Knight
//can make from a certain position is to create an 8x8 board of booleans.
//A square that says true means the knight could move there. False means that
//the knight cannot move there.

//The clear function sets every square to false. This gives us a good starting
//state. If we start with a clear board, we can later go mark the specific squares
//that a knight is allowed to move to.
void clear( board_type& board )
{
    for( auto& row : board )
    {
        for( auto& v : row )
        {
            v = false;
        }
    }
}

//We are going to use some math to determine which squares are possible destinations
//for a knight. We're going to move over 2 rows and 1 column, OR 1 row and 2 columns.
//What if we're in column 0 and try to move over to column -1? That's not valid.
//We must do some bounds-checking. That's what this function is for. It will only
//attempt to set a square to 'true' if it falls within legal bounds for a board.
void set_if_valid( board_type& board, int row, int column )
{
    if( row >= 0 && row < n && column >= 0 && column < n )
    {
        board[row][column] = true ;
    }
}

//This function takes a board, clears it so that everything says false, then manually
//calculates which squares are valid destinations for a knight whose current position is
//specified by the 'row' and 'column' parameters.
void possible_knight_moves( board_type& board, int row, int column )
{
    clear(board) ; //set everything on the board to false

    //check that row and column denote a legal cell on the board
    if( row >= 0 && row < n && column >= 0 && column < n)
    {
        //In many math and science fields, 'delta' is commonly used as a synonym for
        //'amount of change'. Hence, row_delta = 2 means change the row by two and
        //column_delta = 1 means change the column by 1. + and - indicate the direction
        //of change.

        //in these two loops (one nested in the other) we're going to specify all the moves
        //that can happen when we move over TWO rows (in either direction, hence +2 AND -2)
        //and move over ONE column (also in either direction)
        //That's 4 different moves covered by this part.
        for( const int row_delta : { -2, +2 } )
        {
            for( const int column_delta : { -1, +1 } )
            {
                set_if_valid( board, row + row_delta, column + column_delta );
            }
        }

        //in these two loops, now we're going to specify all the moves that can happen when
        //we move over ONE row and TWO columns. Notice that this is the opposite of the last loops.
        //Again, that's 4 different moves covered in this part.
        for( const int row_delta : { -1, +1 } )
        {
            for( const int column_delta : { -2, +2 } )
            {
                set_if_valid( board, row + row_delta, column + column_delta );
            }
        }
    }
    else cout<< "Wrong coordination" <<endl;
}

int main()
{
    board_type board {} ;

    int row = 0;
    int column = 0 ;
    
    cin >> row;
    cout << endl;
    cin >> column ;

    possible_knight_moves( board, row, column ) ;

    for( int row = 0 ; row < n ; ++row ) for( int column = 0 ; column < n ; ++column )
    { if( board[row][column] ) cout << row << ", " << column << '\n' ; }
}


I would suggest making more effort to understand this code than asking for a 'simpler' solution, because this one is pretty concise and straightforward.
Last edited on
Topic archived. No new replies allowed.