Return to Question

I am writing a Tetris program in C# for programming practice, and I've got it working. This is the second post in a multi-part series. My previous post covered the TetrisPiece. You can view the TetrisPiece code and critique here. Future posts may cover the WinForm, the AI, and maybe a graphical board and sounds that I will code later.

I am writing a Tetris program in C# for programming practice, and I've got it working. This is the second post in a multi-part series. My previous post covered the TetrisPiece. You can view the TetrisPiece code and critique here. Future posts may cover the WinForm, the AI, and maybe a graphical board and sounds that I will code later.

I am writing a Tetris program in C# for programming practice, and I've got it working. This is the second post in a multi-part series. My previous post covered the TetrisPiece. You can view the TetrisPiece code and critique here. Future posts may cover the WinForm, the AI, and maybe a graphical board and sounds that I will code later.

These were commented on in the Code Review for the TetrisPiece class, so no need to repeat them here. I will incorporate the feedback when I revise the code for both classes:

Open to any and all feedback. Specific areas that might need improvement include:

• I ended up writing a lot of array manipulation functions, such as FillCharArrayWithChar() and Copy2DCharArray(). Perhaps there are native methods for these already in the C# libraries.
• In the past, it has been suggested that I not use if x == true and if x == false. I find it increases code readability though. Thoughts?
• To test if a move is legal, I make a deep copy of the piece, then move or rotate the TestPiece. If moving/rotating doesn't throw an error, I then set CurrentPiece = TestPiece. Is this the best technique for this?
• I read that using comments is bad, and that code should be written in a way that comments are not needed. Do I have too many comments?
• I read that having a bunch of nested loops and conditionals usually means that the method should be broken into more methods. PlacePiece() has a lot of nested loops and conditionals. Does that mean I should break that method into more methods? Any suggestions on how to do that?
• Should the exception classes be moved into their own file?
• Should the exception classes be shorter? I copied the code from somewhere else, and that code was a bit verbose.
• Other thoughts?

I am writing a Tetris program in C# for programming practice, and I've got it working. This is the second post in a multi-part series. My previous post covered the TetrisPiece. You can view the TetrisPiece code and critique here. Future posts may cover the WinForm, the AI, and maybe a graphical board and sounds that I will code later.

These were commented on in the Code Review for the TetrisPiece class, so no need to repeat them here. I will incorporate the feedback when I revise the code for both classes:

Specific areas that might need improvement include:

• I ended up writing a lot of array manipulation functions, such as FillCharArrayWithChar() and Copy2DCharArray(). Perhaps there are native methods for these already in the C# libraries.
• In the past, it has been suggested that I not use if x == true and if x == false. I find it increases code readability though. Thoughts?
• To test if a move is legal, I make a deep copy of the piece, then move or rotate the TestPiece. If moving/rotating doesn't throw an error, I then set CurrentPiece = TestPiece. Is this the best technique for this?
• I read that using comments is bad, and that code should be written in a way that comments are not needed. Do I have too many comments?
• I read that having a bunch of nested loops and conditionals usually means that the method should be broken into more methods. PlacePiece() has a lot of nested loops and conditionals. Does that mean I should break that method into more methods? Any suggestions on how to do that?
• Should the exception classes be moved into their own file?
• Should the exception classes be shorter? I copied the code from somewhere else, and that code was a bit verbose.

using System;
using System.Diagnostics;

namespace MillenniumTetris
{
    class TetrisBoard
    {
        // TODO: Convert variables to camelCase
        // TODO: Convert public constants to enums.

        #region Variables, Constants, Literals
        private char[,] BoardWithoutCurrentPiece;
        private char[,] BoardWithCurrentPiece;
        private TetrisPiece CurrentPiece;
        // TODO: private TetrisPiece NextPiece;

        private const int BOARD_ROWS = 17;
        private const int BOARD_COLUMNS = 10;
        // TODO: Test different board sizes, especially board widths. Make sure the new pieces
            // come in centered, and that BOARD_ROWS = 4 doesn't throw errors.

        private const bool ENFORCE_TOP_OF_BOARD_BOUNDARY = true;
        private const bool DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY = false;

        public const int CLOCKWISE = 1;
        public const int ONE_HUNDRED_EIGHTY_DEGREES = 2;
        public const int COUNTER_CLOCKWISE = 3;

        public const int LEFT = 1;
        public const int RIGHT = 2;
        public const int DOWN = 3;
        public const int UP = 4;
        #endregion

        #region Public Methods
        public TetrisBoard()
        {
            BoardWithoutCurrentPiece = new char[BOARD_ROWS, BOARD_COLUMNS];

            BoardWithoutCurrentPiece = FillCharArrayWithChar(BoardWithoutCurrentPiece, ' ');

            BoardWithCurrentPiece = Copy2DCharArray(BoardWithoutCurrentPiece);
        }

        public void Tick()
        {
            if (CurrentPiece == null)
            {
                SpawnNewPiece();
            }
            else
            {
                MovePieceDownAndTryToSnap();
            }
        }

        public char[,] GetBoardAsCharArray()
        {
            return BoardWithCurrentPiece;
        }

        public void RotatePiece(int TypeOfRotation)
        {
            TetrisPiece TestPiece = new TetrisPiece(CurrentPiece);
            TestPiece.Rotate(TypeOfRotation);

            try
            {
                PlacePiece(TestPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);

                // if no errors, execute this code to move the piece
                CurrentPiece = TestPiece;
                // TODO: play sound
            }
            catch (Exception ex) when (ex is SquareOccupiedException || ex is SquareOffTheBoardException)
            {
                // If the piece can't be rotated, don't do anything. We don't want to throw a
                // program-ending exception, and we don't want to snap the piece into place.
            }
        }

        public void MovePieceOneSquare(int Direction)
        {
            TetrisPiece TestPiece = new TetrisPiece(CurrentPiece);
            TestPiece.MoveOneSquare(Direction);

            try
            {
                PlacePiece(TestPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);

                // if no errors, execute this code to move the piece
                CurrentPiece = TestPiece;
            }
            catch (Exception ex) when (ex is SquareOccupiedException || ex is SquareOffTheBoardException)
            {
                // If the piece can't be moved, don't do anything. We don't want to throw a
                // program-ending exception, and we don't want to snap the piece into place.
                // To snap the piece into place, use the MovePieceDownAndTryToSnap() method.
            }
        }
        #endregion

        #region Private Methods
        private void MovePieceDownAndTryToSnap()
        {
            TetrisPiece TestPiece = new TetrisPiece(CurrentPiece);
            TestPiece.MoveOneSquare(DOWN);

            try
            {
                PlacePiece(TestPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);

                // if no errors, execute this code to move the piece
                CurrentPiece = TestPiece;
            }
            catch (Exception ex) when (ex is SquareOccupiedException || ex is SquareOffTheBoardException)
            {
                SnapPieceIntoPlace();
            }
        }

        private char[,] Copy2DCharArray(char[,] SourceArray)
        {
            int rows = SourceArray.GetLength(0);
            int columns = SourceArray.GetLength(1);

            char[,] ArrayCopy = new char[rows, columns];

            for (int RowNum = 0; RowNum < rows; RowNum++)
            {
                for (int ColNum = 0; ColNum < columns; ColNum++)
                {
                    ArrayCopy[RowNum, ColNum] = SourceArray[RowNum, ColNum];
                }
            }

            return ArrayCopy;
        }

        private char[,] FillCharArrayWithChar(char[,] ArrayToFill, char CharToFill)
        {
            int rows = ArrayToFill.GetLength(0);
            int columns = ArrayToFill.GetLength(1);

            for (int RowNum = 0; RowNum < rows; RowNum++)
            {
                for (int ColNum = 0; ColNum < columns; ColNum++)
                {
                    ArrayToFill[RowNum, ColNum] = CharToFill;
                }
            }

            return ArrayToFill;
        }

        private void PlacePiece(TetrisPiece Piece, bool EnforceTopOfBoardBoundary)
        {
            // TODO: check for ArgumentException
            // TODO: check shape grid size, make sure it is 5, else throw error
            int[,] Shape = Piece.GetShape();
            int[] Location = Piece.GetLocation();
            char OneLetterName = Piece.GetOneLetterName();

            char[,] TestBoard = Copy2DCharArray(BoardWithoutCurrentPiece);

            for (int row = 0; row < 5; row++)
            {
                for (int column = 0; column < 5; column++)
                {
                    if (Shape[row, column] == 1)
                    {
                        int RowToTry = Location[0] + row;
                        int ColumnToTry = Location[1] + column;

                        if (RowToTry > (BOARD_ROWS - 1) || ColumnToTry > (BOARD_COLUMNS - 1) || ColumnToTry < 0)
                        {
                            throw new SquareOffTheBoardException("Unable to place piece because a target square is off the board.");
                        }
                        else if (RowToTry < 0 && EnforceTopOfBoardBoundary == false)
                        {
                            // Do nothing. This just means the piece is hanging off the top of
                            // the board. This is allowed, but trying to write it to the board
                            // would throw an exception.
                        }
                        else if (RowToTry < 0 && EnforceTopOfBoardBoundary == true)
                        {
                            throw new SquareOffTheBoardException("Unable to place piece because a target square is off the board.");
                        }
                        else if (TestBoard[RowToTry, ColumnToTry] != ' ')
                        {
                            throw new SquareOccupiedException("Unable to place piece because a target square is not empty.");
                        }
                        else
                        {
                            TestBoard[RowToTry, ColumnToTry] = OneLetterName;
                        }
                    }
                }
            }

            // if no errors, go ahead and finalize the piece move
            BoardWithCurrentPiece = Copy2DCharArray(TestBoard);
        }

        private void SnapPieceIntoPlace()
        {
            // TODO: Play sound.
            PlacePiece(CurrentPiece, ENFORCE_TOP_OF_BOARD_BOUNDARY);
            BoardWithoutCurrentPiece = Copy2DCharArray(BoardWithCurrentPiece);

            RemoveFullRows();

            SpawnNewPiece();
        }

        private void RemoveFullRows()
        {
            char[,] BoardChanges = new char[BOARD_ROWS, BOARD_COLUMNS];
            BoardChanges = FillCharArrayWithChar(BoardChanges, ' ');
            bool RowFull;
            bool RowsWereRemoved = false;
            int NextRowToWrite = BOARD_ROWS - 1;

            for (int RowNum = BOARD_ROWS - 1; RowNum >= 0; RowNum--)
            {
                RowFull = true;

                for (int ColNum = 0; ColNum < BOARD_COLUMNS; ColNum++)
                {
                    if (BoardWithCurrentPiece[RowNum, ColNum] == ' ')
                    {
                        RowFull = false;
                        break;
                    }
                }

                if (RowFull == false)
                {
                    BoardChanges = CopyCharArrayRow(BoardWithCurrentPiece, RowNum, BoardChanges, NextRowToWrite);
                    NextRowToWrite--;
                }
                else
                {
                    RowsWereRemoved = true;
                }
            }

            BoardWithCurrentPiece = Copy2DCharArray(BoardChanges);
            BoardWithoutCurrentPiece = Copy2DCharArray(BoardChanges);

            // TODO: Check if RowsWereRemoved = True
                // If so, increment Lines box on WinForm and play a sound
        }

        private char[,] CopyCharArrayRow(char[,] SourceArray, int SourceRow, char[,] DestinationArray, int DestinationRow)
        {
            int SourceArrayRows = SourceArray.GetLength(0);
            int SourceArrayColumns = SourceArray.GetLength(1);
            int DestinationArrayRows = DestinationArray.GetLength(0);
            int DestinationArrayColumns = DestinationArray.GetLength(1);

            if (SourceArrayRows != DestinationArrayRows || SourceArrayColumns != DestinationArrayColumns)
            {
                throw new ArgumentException("Source and destination arrays must be sized identically to perform row copy operation.");
            }

            for ( int ColNum = 0; ColNum < SourceArrayColumns; ColNum++ )
            {
                DestinationArray[DestinationRow,ColNum] = SourceArray[SourceRow,ColNum];
            }

            return DestinationArray;
        }

        private void SpawnNewPiece()
        {
            CurrentPiece = new TetrisPiece();
            CenterNewPiece();

            try
            {
                PlacePiece(CurrentPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);
            }
            catch (SquareOffTheBoardException)
            {
                throw new BoardTooNarrowException("The board is too narrow. Unable to spawn new piece.");
            }
        }

        private void CenterNewPiece()
        {
            int ColumnLocation = (int)Math.Round((BOARD_COLUMNS + 0.5) / 2 - 3, 0);
            CurrentPiece.ChangeLocation(new int[2] { -2, ColumnLocation });
        }
        #endregion
    }
}

#region Exceptions
public class SquareOccupiedException : Exception
{
    public SquareOccupiedException()
    {

    }

    public SquareOccupiedException(string message) : base(message)
    {

    }

    public SquareOccupiedException(string message, Exception inner) : base(message, inner)
    {

    }
}

public class SquareOffTheBoardException : Exception
{
    public SquareOffTheBoardException()
    {

    }

    public SquareOffTheBoardException(string message) : base(message)
    {

    }

    public SquareOffTheBoardException(string message, Exception inner) : base(message, inner)
    {

    }
}

public class BoardTooNarrowException : Exception
{
    public BoardTooNarrowException()
    {

    }

    public BoardTooNarrowException(string message) : base(message)
    {

    }

    public BoardTooNarrowException(string message, Exception inner) : base(message, inner)
    {

    }
}
#endregion

using System;
using System.Diagnostics;

namespace MillenniumTetris
{
    class TetrisBoard
    {
        // TODO: Convert variables to camelCase

        #region Variables, Constants, Literals
        private char[,] BoardWithoutCurrentPiece;
        private char[,] BoardWithCurrentPiece;
        private TetrisPiece CurrentPiece;
        // TODO: private TetrisPiece NextPiece;

        private const int BOARD_ROWS = 17;
        private const int BOARD_COLUMNS = 10;
        // TODO: Test different board sizes, especially board widths. Make sure the new pieces
            // come in centered, and that BOARD_ROWS = 4 doesn't throw errors.

        private const bool ENFORCE_TOP_OF_BOARD_BOUNDARY = true;
        private const bool DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY = false;
        #endregion

        #region Public Methods
        public TetrisBoard()
        {
            BoardWithoutCurrentPiece = new char[BOARD_ROWS, BOARD_COLUMNS];

            BoardWithoutCurrentPiece = FillCharArrayWithChar(BoardWithoutCurrentPiece, ' ');

            BoardWithCurrentPiece = Copy2DCharArray(BoardWithoutCurrentPiece);
        }

        public void Tick()
        {
            if (CurrentPiece == null)
            {
                SpawnNewPiece();
            }
            else
            {
                MovePieceDownAndTryToSnap();
            }
        }

        public char[,] GetBoardAsCharArray()
        {
            return BoardWithCurrentPiece;
        }

        public void RotatePiece(TetrisPiece.Orientation TypeOfRotation)
        {
            TetrisPiece TestPiece = new TetrisPiece(CurrentPiece);
            TestPiece.Rotate(TypeOfRotation);

            try
            {
                PlacePiece(TestPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);

                // if no errors, execute this code to move the piece
                CurrentPiece = TestPiece;
                // TODO: play sound
            }
            catch (Exception ex) when (ex is SquareOccupiedException || ex is SquareOffTheBoardException)
            {
                // If the piece can't be rotated, don't do anything. We don't want to throw a
                // program-ending exception, and we don't want to snap the piece into place.
            }
        }

        public void MovePieceOneSquare(TetrisPiece.Direction myDirection)
        {
            TetrisPiece TestPiece = new TetrisPiece(CurrentPiece);
            TestPiece.MoveOneSquare(myDirection);

            try
            {
                PlacePiece(TestPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);

                // if no errors, execute this code to move the piece
                CurrentPiece = TestPiece;
            }
            catch (Exception ex) when (ex is SquareOccupiedException || ex is SquareOffTheBoardException)
            {
                // If the piece can't be moved, don't do anything. We don't want to throw a
                // program-ending exception, and we don't want to snap the piece into place.
                // To snap the piece into place, use the MovePieceDownAndTryToSnap() method.
            }
        }
        #endregion

        #region Private Methods
        private void MovePieceDownAndTryToSnap()
        {
            TetrisPiece TestPiece = new TetrisPiece(CurrentPiece);
            TestPiece.MoveOneSquare(TetrisPiece.Direction.Down);

            try
            {
                PlacePiece(TestPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);

                // if no errors, execute this code to move the piece
                CurrentPiece = TestPiece;
            }
            catch (Exception ex) when (ex is SquareOccupiedException || ex is SquareOffTheBoardException)
            {
                SnapPieceIntoPlace();
            }
        }

        private char[,] Copy2DCharArray(char[,] SourceArray)
        {
            int rows = SourceArray.GetLength(0);
            int columns = SourceArray.GetLength(1);

            char[,] ArrayCopy = new char[rows, columns];

            for (int RowNum = 0; RowNum < rows; RowNum++)
            {
                for (int ColNum = 0; ColNum < columns; ColNum++)
                {
                    ArrayCopy[RowNum, ColNum] = SourceArray[RowNum, ColNum];
                }
            }

            return ArrayCopy;
        }

        private char[,] FillCharArrayWithChar(char[,] ArrayToFill, char CharToFill)
        {
            int rows = ArrayToFill.GetLength(0);
            int columns = ArrayToFill.GetLength(1);

            for (int RowNum = 0; RowNum < rows; RowNum++)
            {
                for (int ColNum = 0; ColNum < columns; ColNum++)
                {
                    ArrayToFill[RowNum, ColNum] = CharToFill;
                }
            }

            return ArrayToFill;
        }

        private void PlacePiece(TetrisPiece Piece, bool EnforceTopOfBoardBoundary)
        {
            // TODO: check for ArgumentException
            // TODO: check shape grid size, make sure it is 5, else throw error
            int[,] Shape = Piece.Shape;
            int[] Location = Piece.Location;
            char OneLetterName = Piece.OneLetterName;

            char[,] TestBoard = Copy2DCharArray(BoardWithoutCurrentPiece);

            // TODO: refactor to eliminate magic number 5
            for (int row = 0; row < 5; row++)
            {
                for (int column = 0; column < 5; column++)
                {
                    if (Shape[row, column] == 1)
                    {
                        int RowToTry = Location[0] + row;
                        int ColumnToTry = Location[1] + column;

                        if (RowToTry > (BOARD_ROWS - 1) || ColumnToTry > (BOARD_COLUMNS - 1) || ColumnToTry < 0)
                        {
                            throw new SquareOffTheBoardException("Unable to place piece because a target square is off the board.");
                        }
                        else if (RowToTry < 0 && EnforceTopOfBoardBoundary == false)
                        {
                            // Do nothing. This just means the piece is hanging off the top of
                            // the board. This is allowed, but trying to write it to the board
                            // would throw an exception.
                        }
                        else if (RowToTry < 0 && EnforceTopOfBoardBoundary == true)
                        {
                            throw new SquareOffTheBoardException("Unable to place piece because a target square is off the board.");
                        }
                        else if (TestBoard[RowToTry, ColumnToTry] != ' ')
                        {
                            throw new SquareOccupiedException("Unable to place piece because a target square is not empty.");
                        }
                        else
                        {
                            TestBoard[RowToTry, ColumnToTry] = OneLetterName;
                        }
                    }
                }
            }

            // if no errors, go ahead and finalize the piece move
            BoardWithCurrentPiece = Copy2DCharArray(TestBoard);
        }

        private void SnapPieceIntoPlace()
        {
            // TODO: Play sound.
            PlacePiece(CurrentPiece, ENFORCE_TOP_OF_BOARD_BOUNDARY);
            BoardWithoutCurrentPiece = Copy2DCharArray(BoardWithCurrentPiece);

            RemoveFullRows();

            SpawnNewPiece();
        }

        private void RemoveFullRows()
        {
            char[,] BoardChanges = new char[BOARD_ROWS, BOARD_COLUMNS];
            BoardChanges = FillCharArrayWithChar(BoardChanges, ' ');
            bool RowFull;
            bool RowsWereRemoved = false;
            int NextRowToWrite = BOARD_ROWS - 1;

            for (int RowNum = BOARD_ROWS - 1; RowNum >= 0; RowNum--)
            {
                RowFull = true;

                for (int ColNum = 0; ColNum < BOARD_COLUMNS; ColNum++)
                {
                    if (BoardWithCurrentPiece[RowNum, ColNum] == ' ')
                    {
                        RowFull = false;
                        break;
                    }
                }

                if (RowFull == false)
                {
                    BoardChanges = CopyCharArrayRow(BoardWithCurrentPiece, RowNum, BoardChanges, NextRowToWrite);
                    NextRowToWrite--;
                }
                else
                {
                    RowsWereRemoved = true;
                }
            }

            BoardWithCurrentPiece = Copy2DCharArray(BoardChanges);
            BoardWithoutCurrentPiece = Copy2DCharArray(BoardChanges);

            // TODO: Check if RowsWereRemoved = True
                // If so, increment Lines box on WinForm and play a sound
        }

        private char[,] CopyCharArrayRow(char[,] SourceArray, int SourceRow, char[,] DestinationArray, int DestinationRow)
        {
            int SourceArrayRows = SourceArray.GetLength(0);
            int SourceArrayColumns = SourceArray.GetLength(1);
            int DestinationArrayRows = DestinationArray.GetLength(0);
            int DestinationArrayColumns = DestinationArray.GetLength(1);

            if (SourceArrayRows != DestinationArrayRows || SourceArrayColumns != DestinationArrayColumns)
            {
                throw new ArgumentException("Source and destination arrays must be sized identically to perform row copy operation.");
            }

            for ( int ColNum = 0; ColNum < SourceArrayColumns; ColNum++ )
            {
                DestinationArray[DestinationRow,ColNum] = SourceArray[SourceRow,ColNum];
            }

            return DestinationArray;
        }

        private void SpawnNewPiece()
        {
            CurrentPiece = new TetrisPiece();
            CenterNewPiece();

            try
            {
                PlacePiece(CurrentPiece, DO_NOT_ENFORCE_TOP_OF_BOARD_BOUNDARY);
            }
            catch (SquareOffTheBoardException)
            {
                throw new BoardTooNarrowException("The board is too narrow. Unable to spawn new piece.");
            }
        }

        private void CenterNewPiece()
        {
            int ColumnLocation = (int)Math.Round((BOARD_COLUMNS + 0.5) / 2 - 3, 0);
            CurrentPiece.ChangeLocation(new int[2] { -2, ColumnLocation });
        }
        #endregion
    }
}

#region Exceptions
public class SquareOccupiedException : Exception
{
    public SquareOccupiedException()
    {

    }

    public SquareOccupiedException(string message) : base(message)
    {

    }

    public SquareOccupiedException(string message, Exception inner) : base(message, inner)
    {

    }
}

public class SquareOffTheBoardException : Exception
{
    public SquareOffTheBoardException()
    {

    }

    public SquareOffTheBoardException(string message) : base(message)
    {

    }

    public SquareOffTheBoardException(string message, Exception inner) : base(message, inner)
    {

    }
}

public class BoardTooNarrowException : Exception
{
    public BoardTooNarrowException()
    {

    }

    public BoardTooNarrowException(string message) : base(message)
    {

    }

    public BoardTooNarrowException(string message, Exception inner) : base(message, inner)
    {

    }
}
#endregion