Hole05

src/12_RefactoringGolf/hole1/README.md

@@ -0,0 +1,46 @@
+# Hole 5 to Hole 6
+
+Change the code in hole 5 to be identical to the code on hole 6, both implenentation and tests can change.
+
+## Refactorings
+- PROBLEME: la loi de Demeter n'est pas respectée ("Tell, Don't Ask" ou "Holywood Pronciple")
+  - https://savoiragile.com/2012/05/04/connaissez-vous-la-loi-de-demeter/   
+  - https://tech-fr.netlify.app/articles/fr512716/index.html
+  - https://www.arolla.fr/blog/2017/02/principes-solid-vie-de-jours/#Loi_de_Demeter
+  - 
+- REVUE DE CODE: repérez les endroits où pour obtenir une condition dans un IF, on fait appel (ASK) à des propriétés qui ne sont pas dans l'objet courant
+  Example:
+```javascript
+    if (
+         this._board.TileAt(row, firstColumn)!.Symbol ==
+        this._board.TileAt(row, secondColumn)!.Symbol 
+    )
+```
+
+- REFACTORING:
+  - simplifiez les précidicats (les instruction logiques booléennes) en appelant une méthode au niveau de l'objet qui 'possède' les propriétés nécessaires pour évaluer la condition
+
+```javascript
+ if (
+         this._board.TileAt(row, firstColumn)!.hasSameSymbolAs(this._board.TileAt(row, secondColumn))
+    )
+    `....`
+
+  hasSameSymbolAs(other: Tile) {
+    return this.Symbol === other.Symbol;
+  }
+```
+
+## Tips
+
+- Use a diff tool to identify the code changes you need to perform
+- 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

@@ -1,107 +1,142 @@
 /* eslint-disable */
 
+const firstRow = 0;
+const secondRow = 1;
+const thirdRow = 2;
+const firstColumn = 0;
+const secondColumn = 1;
+const thirdColumn = 2;
+
+const playerO = 'O';
+const emptyPlay = ' ';
+
 export class Game {
-  private _lastSymbol = ' ';
-  private _toto: Board = new Board();
+  private _lastSymbol = emptyPlay;
+  private _board: Board = new Board();
 
   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) {
+    if (this._lastSymbol == 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).isNotEmpty) {
       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;
-      }
-    }
+    return this._board.Winner();
+  }
+}
 
-    //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;
-      }
-    }
+class Tile {
+  private x: number = 0;
+  private y: number = 0;
+  private symbol: string = '';
 
-    //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;
-      }
-    }
+  constructor(x: number, y: number, symbol: string) {
+    this.x = x;
+    this.y = y;
+    this.symbol = symbol;
+  }
+
+  get Symbol() {
+    return this.symbol;
+  } 
 
-    return ' ';
+  get isNotEmpty() {
+    return this.symbol !== emptyPlay;
+  }
+
+  hasSameSymbolAs(other: Tile) {
+    return this.symbol === other.symbol;
   }
-}
 
-interface Tile {
-  X: number;
-  Y: number;
-  Symbol: string;
+  hasSameCoordinatesAs(other: Tile) {
+    return this.x == other.x && this.y == other.y;
+  }
+
+  updateSymbol(newSymbol: string) {
+    this.symbol = newSymbol;
+  } 
 }
 
 class Board {
   private _plays: Tile[] = [];
 
   constructor() {
-    for (let i = 0; i < 3; i++) {
-      for (let j = 0; j < 3; j++) {
-        const tile: Tile = { X: i, Y: j, Symbol: ' ' };
-        this._plays.push(tile);
+    for (let i = firstRow; i <= thirdRow; i++) {
+      for (let j = firstColumn; j <= thirdColumn; j++) {
+        this._plays.push(new Tile(i, j, emptyPlay));
       }
     }
   }
 
   public TileAt(x: number, y: number): Tile {
-    return this._plays.find((t: Tile) => t.X == x && t.Y == y)!;
+    return this._plays.find((t: Tile) => t.hasSameCoordinatesAs(new Tile(x, y, emptyPlay)))!; 
   }
 
   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.hasSameCoordinatesAs(new Tile(x, y, symbol)))!
+      .updateSymbol(symbol); 
+  }
+
+  public Winner(): string {
+    if (this.isRowFull(firstRow) && this.isRowFullWithSameSymbol(firstRow)) {
+      return this.TileAt(firstRow, firstColumn)!.Symbol;
+    }
+
+    if (this.isRowFull(secondRow) && this.isRowFullWithSameSymbol(secondRow)) {
+      return this.TileAt(secondRow, firstColumn)!.Symbol;
+    }
+
+    if (this.isRowFull(thirdRow) && this.isRowFullWithSameSymbol(thirdRow)) {
+      return this.TileAt(thirdRow, firstColumn)!.Symbol;
+    }
 
-    this._plays.find((t: Tile) => t.X == x && t.Y == y)!.Symbol = symbol;
+    return emptyPlay;
   }
-}
+
+  private isRowFull(row: number) {
+    return (
+      this.TileAt(row, firstColumn)!.isNotEmpty &&
+      this.TileAt(row, secondColumn)!.isNotEmpty &&
+      this.TileAt(row, thirdColumn)!.isNotEmpty
+    );
+  }
+
+  private isRowFullWithSameSymbol(row: number) {
+    return (
+      this.TileAt(row, firstColumn)!.hasSameSymbolAs(this.TileAt(row, secondColumn)!) &&
+      this.TileAt(row, thirdColumn)!.hasSameSymbolAs(this.TileAt(row, secondColumn)!)
+    );
+  }
+}

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', () => {