3

I'm making a chess game and I was just wondering if I can get rid of these empty catch blocks in my code and have all the try blocks within the same scope go to a single catch block because I think that this is starting to look a little stupid and unorganized.

My Code:

for (int i = 1; i < 8; i++)
{
    try
    {
        if (board[x + i, y].Equals(blackColor))
            checkmateList.AddLast(placementBoard[x + i, y]);//Moving Right
    }
    catch
    {

    }

    try
    {
        if (board[x - i, y].Equals(blackColor))
            checkmateList.AddLast(placementBoard[x - i, y]);//Moving Left
    }
    catch
    {

    }

    try
    {
        if (board[x, y + i].Equals(blackColor))
            checkmateList.AddLast(placementBoard[x, y + i]);//Moving Down
    }
    catch
    {

    }
}

I have empty catch blocks because the only error I get is an out of bounds exception because it will go off the board so I just stop that search and go in a different area but I think that it just looks stupid because a majority of my code that makes the pieces move consist of this.

sujith karivelil
  • 28,671
  • 6
  • 55
  • 88
Doomed Space
  • 31
  • 1
  • 1
  • 8
  • 2
    Why not check the bounds rather than swallow the exception? That'd be a far better approach. – Charles Mager Jan 12 '17 at 16:58
  • 3
    Do not use exceptions for application flow control! – Hackerman Jan 12 '17 at 16:58
  • put the for loop into an exception handler and get rid of the exception handlers inside the for loop. – jdweng Jan 12 '17 at 17:02
  • So Charles you mean like taking the x or y value and subtracting it by 8 and run the loop that many times? – Doomed Space Jan 12 '17 at 17:05
  • Yes, or at the very least check if something is possible **before** you try it. Leave exceptions for things you can't predict, like not having access to the file the user asked you to open. – Jon Hanna Jan 12 '17 at 17:06
  • What exceptions are you expecting to catch? If you are using them to detect index-out-of-bounds conditions then _check that before calling the indexer_. It will be simpler and not have the performance overhead of exceptions. – D Stanley Jan 12 '17 at 17:10
  • Stanley I was not really trying to catch anything, I would run the loop and every time it went through the loop it would add the it just searched to a list, originally I had 4 for loops: 1 for each direction but then I thought it would be easier to just combine all 4 and make 1 loop, this loop checks all directions at once. – Doomed Space Jan 12 '17 at 17:22

3 Answers3

3

Single try with multiple catch is possible whereas we cannot use a catch for more than one try, But you can enclose the whole thing inside a Try-Block with single catch, need not to enclose each statement in separate try blocks. more over continue after getting exception will not be a good practice. Try something like this:

 try
 {
    for (int i = 1; i < 8; i++)
    {

       // all conditions and statements here
    }
 }
 catch
 {
      // handle exception here
 }

In this case we can expect IndexOutOfRangeException, instead for waiting to throw the exception we can simply skip the chance for getting the exception by conditional statements, in such cases the code should be like this:

for (int i = 1; i < 8; i++)
{
    if (x + i < 8 && board[x + i, y].Equals(blackColor))
        checkmateList.AddLast(placementBoard[x + i, y]); //Moving Right

    if (x - i >= 0 && board[x - i, y].Equals(blackColor))
        checkmateList.AddLast(placementBoard[x - i, y]); //Moving Left

    if (y + i < 8 && board[x, y + i].Equals(blackColor))
        checkmateList.AddLast(placementBoard[x, y + i]); //Moving Down
}
sujith karivelil
  • 28,671
  • 6
  • 55
  • 88
  • This will have a different result than the original code though. In the original code if moving left causes an exception it will still move right and move down. You're code will exit as soon as one of the moves fail. – gunnerone Jan 12 '17 at 17:06
  • Thanks I have updated the answer. You need not to get the bounds since it will always be `8`, since it is a chess board – sujith karivelil Jan 12 '17 at 17:17
  • Thanks, I just recently typed the code that I posted because instead of finding the king and searching for pieces that cause checkmate, I instead made an algorithm to search for each pieces move set and if it lead to the king it would run the code that I made for checkmate but that code was just god awful. It was a good 1000 lines give or take so I decided to scrap it and start fresh. And I just now realized I never incremented x or y so both variables just sat at 0 – Doomed Space Jan 12 '17 at 17:40
2

As others have mentioned it would be better to remove the try/catches and instead test if the move is on the board.

for (int i = 1; i < 8; i++)
{
    if (x + i < board.GetLength(0) && board[x + i, y].Equals(blackColor))
        checkmateList.AddLast(placementBoard[x + i, y]); //Moving Right

    if (x - i >= 0 && board[x - i, y].Equals(blackColor))
        checkmateList.AddLast(placementBoard[x - i, y]); //Moving Left

    if (y + i < board.GetLength(1) && board[x, y + i].Equals(blackColor))
        checkmateList.AddLast(placementBoard[x, y + i]); //Moving Down
}
gunnerone
  • 3,566
  • 2
  • 14
  • 18
1

Exceptions for flow control = bad idea. They're for finding problems, not causing them.

To answer the original question, you can nest your try/catch blocks.

try{  try{ condition; } catch { response(); } catch {response2();}

Now, lets do it the right way instead. Outside of the method, let's do:

        public enum Directions 
        {
            Right, 
            Left, 
            Down
        }

Now, let's use it.

for (int i = 1; i < 8; i++)
{
    bool right, left, down;
     right =  board[x + i, y].Equals(blackColor);
     left = board[x + i, y].Equals(blackColor);
     down = board[x - i,y].Equals(blackColor)    
    var direction = new List<bool>() { right, left, down }


    for(i = 0; i < direction.Count, i++) 
    {
      try    
       {  
             if(direction[i])
             {
                  switch(i)
                  {
                     case Directions.Right: checkmateList.AddLast(placementBoard[x + i, y]); break;
                     case Directions.Left: checkmateList.AddLast(placementBoard[x - i, y];); break;
                     case Directions.Down: checkmateList.AddLast(placementBoard[x, y + i]); break;
                     default:
                            throw new ApplicationException("oops");
                            break; //not needed but i prefer to keep breaks in anyway       
                  }
             }  
       } catch (Exception ex){ //do stuff; }
    }

}
Not my prettiest formatting but there you go. Indeces and enums can make great pals done right.

CDove
  • 1,940
  • 10
  • 19
  • I have no idea how to use enums and I just literally learned about switchs like 2 days ago, also I only posted 3 moves just as a sample, I have every move of each piece here that i'm working with so that's going to be a lot of booleans not to mention that I also have to use the white pieces as well. – Doomed Space Jan 12 '17 at 17:47
  • An enum basically names a number. In the code I posted, I put the booleans into a list of possibilities and intentionally matched the index in the list to the number represented by the enum (Directions.Right == 0). Since in Chess there are many types of move, I recommend that you create a class called Moves and put your move style logic there. Then you can call, say, Moves.Knight.ForwardRight() and the piece moves forward two and right one. Moves.Queen.DownLeft(x) could move a queen x number of spaces. – CDove Jan 12 '17 at 17:53
  • I have classes for each piece that holds does their moves – Doomed Space Jan 12 '17 at 17:57