Bugfix/exo04

src/12_RefactoringGolf/hole1/README.md

@@ -0,0 +1,35 @@
+# Exo 4 to Exo 5
+
+
+
+## Refactorings
+
+- Tackle scope of constants and variables
+  - Move constants and variables to proper scope
+  - les constantes et les variables ne sont probablement pas au bon endroit, dans le bon scope
+    - https://www.tektutorialshub.com/typescript/variable-scope-in-typescript/
+    - The Typescript variable can be in three scopes depending on how & where you have defined them.
+      - Global Scope
+      - Function Scope or Class Scope
+      - Local Scope or Block scope
+  - Déplacez les pour qu'elles ne soient pas globales mais utilisées aux seuls endroits où on en a besoin
+
+- Tackle duplication
+  - Introduce more generic methods to replace duplicated methods
+  - il y a des méthodes qui font la meme chose, trouvez les, et en les rendant à peine plus générique, réduisez le code dupliqué au strict nécessaire.
+
+## Tips
+
+- Use a diff tool to identify the code changes you need to perform
+-  https://devconnected.com/how-to-compare-two-git-branches/
+- $ git diff branch1..branch2
+- Check the code coverage is enough to detect any unintended behaviour changes
+
+### While refactoring
+
+- Stay in the green while refactoring
+  - Run the tests after each refactor
+    - Check all tests still pass
+    - Check code coverage has not dropped
+- Commit after each refactor
+- In case of persistent test fails, use `git reset` to go back to green

src/12_RefactoringGolf/hole1/kata.ts

@@ -2,77 +2,120 @@
 
 export class Game {
   private _lastSymbol = ' ';
-  private _toto: Board = new Board();
+  private _board: Board = new Board();
+
+  private readonly emptyPlay = ' ';
+
+  private readonly firstRow = 0;
+  private readonly secondRow = 1;
+  private readonly thirdRow = 2;
+  private readonly firstColumn = 0;
+  private readonly secondColumn = 1;
+  private readonly thirdColumn = 2;
 
   public Play(symbol: string, x: number, y: number): void {
-    //if first move
-    if (this._lastSymbol == ' ') {
-      //if player is X
-      if (symbol == 'O') {
+    this.validateFirstMove(symbol);
+    this.validatePlayer(symbol);
+    this.validatePositionIsEmpty(x, y);
+
+    this.updateLastPlayer(symbol);
+    this.updateBoard(symbol, x, y);
+  }
+
+  private validateFirstMove(player: string) {
+    let playerO = '0'
+    if (this._lastSymbol == this.emptyPlay) {
+      if (player == playerO) {
         throw new Error('Invalid first player');
       }
     }
-    //if not first move but player repeated
-    else if (symbol == this._lastSymbol) {
+  }
+
+  private validatePlayer(player: string) {
+    if (player == this._lastSymbol) {
       throw new Error('Invalid next player');
     }
-    //if not first move but play on an already played tile
-    else if (this._toto.TileAt(x, y).Symbol != ' ') {
+  }
+
+  private validatePositionIsEmpty(x: number, y: number) {
+    if (this._board.TileAt(x, y).Symbol != this.emptyPlay) {
       throw new Error('Invalid position');
     }
+  }
 
-    // update game state
-    this._lastSymbol = symbol;
-    this._toto.AddTileAt(symbol, x, y);
+  private updateLastPlayer(player: string) {
+    this._lastSymbol = player;
+  }
+
+  private updateBoard(player: string, x: number, y: number) {
+    this._board.AddTileAt(player, x, y);
   }
 
   public Winner(): string {
-    //if the positions in first row are taken
-    if (
-      this._toto.TileAt(0, 0)!.Symbol != ' ' &&
-      this._toto.TileAt(0, 1)!.Symbol != ' ' &&
-      this._toto.TileAt(0, 2)!.Symbol != ' '
-    ) {
-      //if first row is full with same symbol
-      if (
-        this._toto.TileAt(0, 0)!.Symbol == this._toto.TileAt(0, 1)!.Symbol &&
-        this._toto.TileAt(0, 2)!.Symbol == this._toto.TileAt(0, 1)!.Symbol
-      ) {
-        return this._toto.TileAt(0, 0)!.Symbol;
-      }
+    if (this.isFirstRowFull() && this.isFirstRowFullWithSameSymbol()) {
+      return this._board.TileAt(this.firstRow, this.firstColumn)!.Symbol;
     }
 
-    //if the positions in first row are taken
-    if (
-      this._toto.TileAt(1, 0)!.Symbol != ' ' &&
-      this._toto.TileAt(1, 1)!.Symbol != ' ' &&
-      this._toto.TileAt(1, 2)!.Symbol != ' '
-    ) {
-      //if middle row is full with same symbol
-      if (
-        this._toto.TileAt(1, 0)!.Symbol == this._toto.TileAt(1, 1)!.Symbol &&
-        this._toto.TileAt(1, 2)!.Symbol == this._toto.TileAt(1, 1)!.Symbol
-      ) {
-        return this._toto.TileAt(1, 0)!.Symbol;
-      }
+    if (this.isSecondRowFull() && this.isSecondRowFullWithSameSymbol()) {
+      return this._board.TileAt(this.secondRow, this.firstColumn)!.Symbol;
     }
 
-    //if the positions in first row are taken
-    if (
-      this._toto.TileAt(2, 0)!.Symbol != ' ' &&
-      this._toto.TileAt(2, 1)!.Symbol != ' ' &&
-      this._toto.TileAt(2, 2)!.Symbol != ' '
-    ) {
-      //if middle row is full with same symbol
-      if (
-        this._toto.TileAt(2, 0)!.Symbol == this._toto.TileAt(2, 1)!.Symbol &&
-        this._toto.TileAt(2, 2)!.Symbol == this._toto.TileAt(2, 1)!.Symbol
-      ) {
-        return this._toto.TileAt(2, 0)!.Symbol;
-      }
+    if (this.isThirdRowFull() && this.isThirdRowFullWithSameSymbol()) {
+      return this._board.TileAt(this.thirdRow, this.firstColumn)!.Symbol;
     }
 
-    return ' ';
+    return this.emptyPlay;
+  }
+
+  private isFirstRowFull() {
+    return (
+      this._board.TileAt(this.firstRow, this.firstColumn)!.Symbol != this.emptyPlay &&
+      this._board.TileAt(this.firstRow, this.secondColumn)!.Symbol != this.emptyPlay &&
+      this._board.TileAt(this.firstRow, this.thirdColumn)!.Symbol != this.emptyPlay
+    );
+  }
+
+  private isFirstRowFullWithSameSymbol() {
+    return (
+      this._board.TileAt(this.firstRow, this.firstColumn)!.Symbol ==
+        this._board.TileAt(this.firstRow, this.secondColumn)!.Symbol &&
+      this._board.TileAt(this.firstRow, this.thirdColumn)!.Symbol ==
+        this._board.TileAt(this.firstRow, this.secondColumn)!.Symbol
+    );
+  }
+
+  private isSecondRowFull() {
+    return (
+      this._board.TileAt(this.secondRow, this.firstColumn)!.Symbol != this.emptyPlay &&
+      this._board.TileAt(this.secondRow, this.secondColumn)!.Symbol != this.emptyPlay &&
+      this._board.TileAt(this.secondRow, this.thirdColumn)!.Symbol != this.emptyPlay
+    );
+  }
+
+  private isSecondRowFullWithSameSymbol() {
+    return (
+      this._board.TileAt(this.secondRow, this.firstColumn)!.Symbol ==
+        this._board.TileAt(this.secondRow, this.secondColumn)!.Symbol &&
+      this._board.TileAt(this.secondRow, this.thirdColumn)!.Symbol ==
+        this._board.TileAt(this.secondRow, this.secondColumn)!.Symbol
+    );
+  }
+
+  private isThirdRowFull() {
+    return (
+      this._board.TileAt(this.thirdRow, this.firstColumn)!.Symbol != this.emptyPlay &&
+      this._board.TileAt(this.thirdRow, this.secondColumn)!.Symbol != this.emptyPlay &&
+      this._board.TileAt(this.thirdRow, this.thirdColumn)!.Symbol != this.emptyPlay
+    );
+  }
+
+  private isThirdRowFullWithSameSymbol() {
+    return (
+      this._board.TileAt(this.thirdRow, this.firstColumn)!.Symbol ==
+        this._board.TileAt(this.thirdRow, this.secondColumn)!.Symbol &&
+      this._board.TileAt(this.thirdRow, this.thirdColumn)!.Symbol ==
+        this._board.TileAt(this.thirdRow, this.secondColumn)!.Symbol
+    );
   }
 }
 
@@ -99,9 +142,6 @@ class Board {
   }
 
   public AddTileAt(symbol: string, x: number, y: number): void {
-    //@ts-ignore
-    const tile: Tile = { X: x, Y: y, Symbol: symbol };
-
     this._plays.find((t: Tile) => t.X == x && t.Y == y)!.Symbol = symbol;
   }
 }

src/12_RefactoringGolf/hole1/test.ts

@@ -8,23 +8,31 @@ describe('TicTacToe game', () => {
   });
 
   test('should not allow player O to play first', () => {
-    expect(() => game.Play('O', 0, 0)).toThrow();
+    expect(() => {
+      game.Play('O', 0, 0);
+    }).toThrow();
   });
 
   it('should not allow player x to play twice in a row', () => {
     game.Play('X', 0, 0);
-    expect(() => game.Play('X', 1, 0)).toThrow();
+    expect(() => {
+      game.Play('X', 1, 0);
+    }).toThrow();
   });
 
   it('should not allow a player to play in last played position', () => {
     game.Play('X', 0, 0);
-    expect(() => game.Play('O', 0, 0)).toThrow();
+    expect(() => {
+      game.Play('O', 0, 0);
+    }).toThrow();
   });
 
   it('should not allow a player to play in any played position', () => {
     game.Play('X', 0, 0);
     game.Play('O', 1, 0);
-    expect(() => game.Play('X', 0, 0)).toThrow();
+    expect(() => {
+      game.Play('X', 0, 0);
+    }).toThrow();
   });
 
   it('should declare player X as winner if it plays three in top row', () => {