diff --git a/src/12_RefactoringGolf/hole1/README.md b/src/12_RefactoringGolf/hole1/README.md new file mode 100644 index 0000000..b28a046 --- /dev/null +++ b/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 diff --git a/src/12_RefactoringGolf/hole1/kata.ts b/src/12_RefactoringGolf/hole1/kata.ts index c750e5e..669cff0 100644 --- a/src/12_RefactoringGolf/hole1/kata.ts +++ b/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; } } diff --git a/src/12_RefactoringGolf/hole1/test.ts b/src/12_RefactoringGolf/hole1/test.ts index 44d6c2a..9aa2e2e 100644 --- a/src/12_RefactoringGolf/hole1/test.ts +++ b/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', () => { diff --git a/src/README.md b/src/README.md deleted file mode 100644 index 4cfc9a0..0000000 --- a/src/README.md +++ /dev/null @@ -1,22 +0,0 @@ -# Hole 1 to Hole 2 - -Change the code in hole 1 to be identical to the code in hole 2; implementation and tests can change. - -## Refactorings - -- Tackle code comments, long method and large class - - Extract method - -## 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; no failing tests - - 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 compilation errors or test fails, use `git reset` to go back to green