Ludo Refactoring — merge Game and Board

TL;DR

We merge the Game and Board classes. Although they have slightly different responsibilities, the Board is just a lower-level view of the Game. The only client of the Board is the Game.

Refactoring strategy

We can see the game as a specialization of the Board, so in a first step, we eliminate any naming conflicts and make the Game a subclass of the Board. (Actually three steps: eliminate naming conflicts, make it a subclass, and run the tests.)

Next we try to push up methods from Game to Board.

Then we remove references to Game, and remove the Game class.

Finally we rename Board to LudoGame.

We start with this version of the Ludo game from 2022-05-04.

Eliminate naming conflicts

Analysis

Here we have the common selectors.

Caveat: of course this won't work now, because in this image the classes have already been merged, and GtLudoBoard no longer exists. However you can browse the github snapshot above to get an insight into what the original state was.

common := GtLudoGame selectors asSet & GtLudoBoard selectors asSet
  

Let's review the Game methods also in the Board:

GtLudoGame methods select: [ :m | common includes: m selector ]
  

The gt* view methods are just forwards, so we can ignore them. (I.e., remove them when we subclass Game from Board.)

Similarly, the feedback:, die, and roll: methods just forward to the board, so we can remove them.

This leaves initialize and players.

The players method is special, since Game uses the players slot to implement a queue of players to play, whereas the Board assumes the players are in a fixed order. We should rename the players slot in the Game to, e.g., playerQueue.

The two initialize methods need to be merged. First we can compose them with a super initialize. We should take care with the board slot. We can initialize it to self, and later deprecate and then remove the board accessor.

Steps

We delete gt*, feedback:, die, and roll: from GtLudoGame Object subclass: #GtLudoGame instanceVariableNames: 'players squares startSquares goalSquares die announcer feedback winner needToRollDie lastDieRolled playerQueue routeCache' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Model' .

We rename the players slot to playerQueue.

We try to rename the players accessor in GtLudoGame Object subclass: #GtLudoGame instanceVariableNames: 'players squares startSquares goalSquares die announcer feedback winner needToRollDie lastDieRolled playerQueue routeCache' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Model' , but this would also rename other players methods in other classes, namely in GtLudoBoard, so we must refactor manually.

We add a new playerQueue accessor and remove the players accessor.

We search for senders of players within GtLudoGame Object subclass: #GtLudoGame instanceVariableNames: 'players squares startSquares goalSquares die announcer feedback winner needToRollDie lastDieRolled playerQueue routeCache' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Model' .

#players gtSenders & GtLudoGame gtMethodsInClass
  

We manually rename each players send to a playerQueue send.

This leaves the initialize method.

As a quick solution, we set board to self. Later we can replace all sends to board or self board to self and remove the board slot and accessor.

We also perform a super initialize.

We check that there are no more common methods except for initialize, and no more sends to players except in the initialize method.

GtLudoGame selectors asSet & GtLudoBoard selectors asSet
  

#players gtSenders & GtLudoGame gtMethodsInClass
  

Now we should be ready to make the Game a subclass of the Board

Turning the Game into a subclass of the Board

We change the GtLudoGame Object subclass: #GtLudoGame instanceVariableNames: 'players squares startSquares goalSquares die announcer feedback winner needToRollDie lastDieRolled playerQueue routeCache' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Model' definition to make it a subclass of GtLudoBoard.

We check that the warnings in GtLudoGame>>#initialize initialize super initialize. announcer := Announcer new. feedback := ''. players := OrderedCollection new. ($A to: $D) do: [ :player | players add: (GtLudoPlayer new name: player) ]. self initializeSquares. players do: #initializeTokens. self initializeStartSquares. self initializeGoalSquares. die := GtLudoDie new. routeCache := Dictionary new. "Initialization code from the Game class" self die announcer when: GtLudoDieRolled send: #onRolled to: self. lastDieRolled := self die topFace. self squares , self startSquares do: [ :square | square announcer when: GtLudoSquareSignalTokenMove do: [ :announcement | self moveToken: announcement token ] ]. playerQueue := self players copy. playerQueue do: [ :each | each game: self ]. self requireDieRoll. self setWinner. self feedback: self gameState are now resolved, as feedback: and die are now inherited.

We run all the tests in GtLudoBoardExamples Object subclass: #GtLudoBoardExamples instanceVariableNames: '' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Examples' and GtLudoGameExamples Object subclass: #GtLudoGameExamples instanceVariableNames: '' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Examples' .

examples := GtExplicitExampleGroup withAll: GtLudoGame package gtExamplesAllContained.
examples runAll
  

All tests are green, so now lets'clean up the references to board.

We change all sends to self board into self, and we remove the board slot and accessors.

#board gtSenders & GtLudoGame gtMethodsInClass
  

The refactoring engine won't let us remove the slot, so we search for any remaining references:

#board gtReferences
  

We rename the slot and try to remove it, but it still thinks there are references. We'll worry about that later.

#boardNOTNEEDED gtReferences
  

We run all the tests again. This time the tests fail, and we find the dangling references to the slot, which is not initialized. We replace it by self and remove the slot.

The tests are all green now. We can commit this version.

Merging the Game and the Board

Now we want to merge the two classes. We start by pushing all the slots of GtLudoGame Object subclass: #GtLudoGame instanceVariableNames: 'players squares startSquares goalSquares die announcer feedback winner needToRollDie lastDieRolled playerQueue routeCache' classVariableNames: '' package: 'GToolkit-Demo-Ludo-Model' to the superclass.

We can review the changes first:

namespace := RBNamespace new.
(GtLudoGame slots collect: [ :each | each name asSymbol ])
	do:
		[ :each | 
		(RBPullUpInstanceVariableRefactoring
			model: namespace
			variable: each
			class: GtLudoGame superclass) primitiveExecute ].
namespace
  

And now do it.

(GtLudoGame slots collect: [ :each | each name asSymbol ])
	do: [ :each | (RBPullUpInstanceVariableRefactoring variable: each class: GtLudoGame superclass) execute ]
  

The tests are green!

Now let's move up all the methods except initialize.

methodsToPushUp := GtLudoGame selectors reject: [:each | each = #initialize]
  

Let's inspect the changes:

namespace := RBNamespace new.
(RBPullUpMethodRefactoring
	model: namespace
	pullUp: methodsToPushUp
	from: GtLudoGame) primitiveExecute.
namespace
  

Looks good. Let's do it.

(RBPullUpMethodRefactoring pullUp: methodsToPushUp from: GtLudoGame) execute
  

(Actually this raises an error (Instance of AsyncStreamTransitionBuffer did not understand #preloadAmount:) but it works in the Morphic browser for now.

We run the tests.

Now we just have to fix the initialize method. We copy up the Game initialization and run the tests

Finally we replace all references to GtLudoGame to refer to the Board instaed.

GtLudoGame gtReferences
  

There is only one (in the examples) so we make it refer to the Board.

We remove the Game class and run the tests.

We rename Board to Game.

The tests are green, so we commit this version.

Next: Refactoring scenario with Ludo