diff --git a/Tetris.ConsoleUI/ConsoleDrawing.cs b/Tetris.ConsoleUI/ConsoleDrawing.cs
index 8e7ad62..ba18abc 100644
--- a/Tetris.ConsoleUI/ConsoleDrawing.cs
+++ b/Tetris.ConsoleUI/ConsoleDrawing.cs
@@ -24,6 +24,9 @@ class ConsoleDrawing : IDrawing
public void DrawScene(Game game)
{
+ /*
+ * Reivew VV: для чого тут слід використовувати критичну секцію?
+ */
lock (game)
{
//Current Position
@@ -92,6 +95,9 @@ public void ShowGameOver(Game game)
}
}
+ /*
+ * Review VV: чому ця функція статична?
+ */
private static void ShowLines(Game game)
{
Console.CursorLeft = game.ActualBoard.Width + 4;
@@ -99,6 +105,9 @@ private static void ShowLines(Game game)
Console.WriteLine("Lines: {0}", game.Lines);
}
+ /*
+ * Review VV: чому ця функція статична?
+ */
private static void ShowScore(Game game)
{
Console.CursorLeft = game.ActualBoard.Width + 4;
@@ -168,6 +177,10 @@ private void DrawArray(int[,] arr, bool border)
}
}
+
+ /*
+ * Review VV: чому ця функція статична?
+ */
///
/// Writes a String in the Specified foreground color,
/// then switches the Color back
diff --git a/Tetris.ConsoleUI/Program.cs b/Tetris.ConsoleUI/Program.cs
index 1ef0165..c450c53 100644
--- a/Tetris.ConsoleUI/Program.cs
+++ b/Tetris.ConsoleUI/Program.cs
@@ -21,6 +21,10 @@ static int Main(string[] args)
drawer = new ConsoleDrawing();
+ /*
+ * Review VV: чому тут використовується статична функція?
+ * Чи можна це замінити на drawer.ShowControls()?
+ */
ConsoleDrawing.ShowControls();
Console.ReadKey(true);
diff --git a/Tetris.GameEngine.Test/PieceFactoryTest.cs b/Tetris.GameEngine.Test/PieceFactoryTest.cs
index 2792a62..568ccef 100644
--- a/Tetris.GameEngine.Test/PieceFactoryTest.cs
+++ b/Tetris.GameEngine.Test/PieceFactoryTest.cs
@@ -41,6 +41,9 @@ public void TestGetRandomPiece()
Assert.IsNotNull(gen_piece);
}
+ /*
+ * Review VV: цей метод повністю повторює функціональність першого - TestInitialization()
+ */
[TestMethod]
public void TestGetCount()
{
diff --git a/Tetris.GameEngine/Board.cs b/Tetris.GameEngine/Board.cs
index 4202583..4997e7b 100644
--- a/Tetris.GameEngine/Board.cs
+++ b/Tetris.GameEngine/Board.cs
@@ -27,6 +27,7 @@ public Board(int width, int height)
}
else
{
+ // Reivew VV: рекомендую валідацію писати на початку методу і брати в регіон "Valiation"
throw new ArgumentException("Board must be at least 10x20");
}
}
@@ -144,6 +145,8 @@ public int Height
{
get
{
+ // Review VV: чому тут слід використовувати Array.GetUpperBound()?
+ // Чи є альтернативни цій функції?
return _mBoard.GetUpperBound(0) + 1;
}
}
@@ -152,6 +155,8 @@ public int Width
{
get
{
+ // Review VV: чому тут слід використовувати Array.GetUpperBound()?
+ // Чи є альтернативни цій функції?
return _mBoard.GetUpperBound(1) + 1;
}
}
diff --git a/Tetris.GameEngine/Game.cs b/Tetris.GameEngine/Game.cs
index 8fcaae5..015bd53 100644
--- a/Tetris.GameEngine/Game.cs
+++ b/Tetris.GameEngine/Game.cs
@@ -5,6 +5,7 @@ namespace Tetris.GameEngine
{
public class Game: IMovable, IMode
{
+ // Review VV: рекомендую замінити "static readonly" на "const"
private static readonly int _board_width = 10;
private static readonly int _board_height = 20;
@@ -21,6 +22,7 @@ public enum GameStatus
///
/// The Playfield
///
+ // Review VV: невірне іменування: слід замінити на _gameBoard
private Board _game_board;
private GameStatus _status;
private Piece _currPiece;
@@ -66,6 +68,9 @@ public void Start()
public void Pause()
{
+ /*
+ * Review VV: чи можна викликати цей метод, якщо гра знаходиться у статусах ReadyToStart чи Finished?
+ */
if (this._status == GameStatus.InProgress)
{
this._status = GameStatus.Paused;
@@ -105,6 +110,11 @@ public int PosY
}
}
+ /*
+ * Reivew VV: What does this method do?
+ * Is it correct to use Clone()?
+ * Is there more optimal approach?
+ */
public Board ActualBoard
{
get
@@ -253,11 +263,17 @@ public void Rotate()
#region IMode Implementation
+ /*
+ * Review VV: what is the purpose of it?
+ */
public bool NextPieceMode
{
get; set;
}
+ /*
+ * Review VV: what is the purpose of it?
+ */
public bool ShadowPieceMode
{
get; set;
diff --git a/Tetris.GameEngine/Interfaces/IDrawing.cs b/Tetris.GameEngine/Interfaces/IDrawing.cs
index 1b6273c..d0b1983 100644
--- a/Tetris.GameEngine/Interfaces/IDrawing.cs
+++ b/Tetris.GameEngine/Interfaces/IDrawing.cs
@@ -2,6 +2,10 @@
namespace Tetris.GameEngine.Interfaces
{
+ /*
+ * Review VV: думаю, цей інтерфейс не повинен бути у зборці Tetris.GameEngine.
+ * Оскільки ця зборка відповідає тільки за логіку гри і не повинна відповідати за відображення.
+ */
public interface IDrawing
{
void DrawScene(Game game);
diff --git a/Tetris.GameEngine/Piece.cs b/Tetris.GameEngine/Piece.cs
index b6aa331..4b3a388 100644
--- a/Tetris.GameEngine/Piece.cs
+++ b/Tetris.GameEngine/Piece.cs
@@ -5,7 +5,11 @@ namespace Tetris.GameEngine
public class Piece: ICloneable
{
#region Private Fields
-
+ /*
+ * Review VV: слід додати модифікатор доступу 'private'.
+ * Це питання стилю.
+ * Я рекомендую завжди явно вказувати специфікатор доступу.
+ */
int[,] _piece;
int _initPosX;
int _initPosY;
@@ -20,6 +24,7 @@ public Piece(int[,] p)
{
throw new NullReferenceException();
}
+ // Reive VV: для чого потрібно клонувати цей масив?
_piece = (int[,])p.Clone();
_initPosY = (p.GetUpperBound(0) + 1) * -1;
_initPosX = 0;
diff --git a/Tetris.GameEngine/PieceFactory.cs b/Tetris.GameEngine/PieceFactory.cs
index f89aa65..fbb35b3 100644
--- a/Tetris.GameEngine/PieceFactory.cs
+++ b/Tetris.GameEngine/PieceFactory.cs
@@ -3,6 +3,16 @@
namespace Tetris.GameEngine
{
+ /*
+ * Review VV:
+ * - виділення окремої фабрики для фігурок - правильне рішення
+ *
+ * Рекомендую скористатися принципом Dependency Inversion:
+ * - реалізувати фабрику як нестатичний клас
+ * - виділити для неї інтерфейс IPieceFactory
+ * - передавати екземпляр фабрики як параметр конструктора у Game
+ * - але список _pieces варто залишити статичним
+ */
public static class PieceFactory
{
#region Private Fields
@@ -33,6 +43,10 @@ static PieceFactory()
///
/// ID of Piece (0-6)
/// the Piece (or null if invalid Value)
+ ///
+ /*
+ Review VV: ID повинно бути маленькими літерами - id
+ */
public static Piece GetPiecebyId(int ID)
{
if (_pieces.Count > ID && ID >= 0)