Complex classes like Round often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use Round, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
15 | class Round |
||
16 | { |
||
17 | /** |
||
18 | * @var Table |
||
19 | */ |
||
20 | private $table; |
||
21 | |||
22 | /** |
||
23 | * @var ChipStackCollection |
||
24 | */ |
||
25 | private $betStacks; |
||
26 | |||
27 | /** |
||
28 | * @var PlayerCollection |
||
29 | */ |
||
30 | private $foldedPlayers; |
||
31 | |||
32 | /** |
||
33 | * @var ChipPotCollection |
||
34 | */ |
||
35 | private $chipPots; |
||
36 | |||
37 | /** |
||
38 | * @var ChipPot |
||
39 | */ |
||
40 | private $currentPot; |
||
41 | |||
42 | /** |
||
43 | * @var ActionCollection |
||
44 | */ |
||
45 | private $playerActions; |
||
46 | |||
47 | /** |
||
48 | * @var PlayerCollection |
||
49 | */ |
||
50 | private $leftToAct; |
||
51 | |||
52 | /** |
||
53 | * @var GameParameters |
||
54 | */ |
||
55 | private $gameRules; |
||
56 | |||
57 | /** |
||
58 | * Round constructor. |
||
59 | * |
||
60 | * @param Table $table |
||
61 | * @param GameParameters $gameRules |
||
62 | */ |
||
63 | private function __construct(Table $table, GameParameters $gameRules) |
||
84 | 50 | ||
85 | 50 | /** |
|
86 | 50 | * Start a Round of poker. |
|
87 | 50 | * |
|
88 | 50 | * @param Table $table |
|
89 | * @param GameParameters $gameRules |
||
90 | * |
||
91 | 50 | * @return Round |
|
92 | */ |
||
93 | public static function start(Table $table, GameParameters $gameRules): Round |
||
97 | 50 | ||
98 | 50 | /** |
|
99 | 50 | * Run the cleanup procedure for an end of Round. |
|
100 | */ |
||
101 | public function end() |
||
111 | |||
112 | /** |
||
113 | * @return DealerContract |
||
114 | */ |
||
115 | public function dealer(): DealerContract |
||
119 | |||
120 | 11 | /** |
|
121 | * @return PlayerCollection |
||
122 | 11 | */ |
|
123 | public function players(): PlayerCollection |
||
127 | |||
128 | /** |
||
129 | * @return PlayerCollection |
||
130 | 50 | */ |
|
131 | public function playersStillIn(): PlayerCollection |
||
135 | |||
136 | /** |
||
137 | * @return PlayerCollection |
||
138 | 3 | */ |
|
139 | public function foldedPlayers(): PlayerCollection |
||
143 | |||
144 | /** |
||
145 | * @return ActionCollection |
||
146 | 12 | */ |
|
147 | public function playerActions(): ActionCollection |
||
151 | |||
152 | /** |
||
153 | * @return LeftToAct |
||
154 | 19 | */ |
|
155 | public function leftToAct(): LeftToAct |
||
159 | |||
160 | /** |
||
161 | * @return Table |
||
162 | 1 | */ |
|
163 | public function table(): Table |
||
167 | |||
168 | /** |
||
169 | * @return ChipStackCollection |
||
170 | 35 | */ |
|
171 | public function betStacks(): ChipStackCollection |
||
175 | |||
176 | /** |
||
177 | * @return GameParameters |
||
178 | 36 | */ |
|
179 | public function gameRules(): GameParameters |
||
183 | |||
184 | /** |
||
185 | * @return int |
||
186 | 50 | */ |
|
187 | public function betStacksTotal(): int |
||
191 | |||
192 | public function dealHands() |
||
200 | |||
201 | /** |
||
202 | 29 | * Runs over each chipPot and assigns the chips to the winning player. |
|
203 | */ |
||
204 | 29 | private function distributeWinnings() |
|
250 | 11 | ||
251 | /** |
||
252 | 11 | * @param Player $actualPlayer |
|
253 | * |
||
254 | 11 | * @return bool |
|
255 | */ |
||
256 | public function playerIsStillIn(PlayerContract $actualPlayer) |
||
262 | 1 | ||
263 | /** |
||
264 | 1 | * @return PlayerContract |
|
265 | 1 | */ |
|
266 | public function playerWithButton(): PlayerContract |
||
270 | |||
271 | 11 | /** |
|
272 | * @return PlayerContract |
||
273 | */ |
||
274 | public function playerWithSmallBlind(): PlayerContract |
||
282 | 1 | ||
283 | /** |
||
284 | * @return PlayerContract |
||
285 | */ |
||
286 | public function playerWithBigBlind(): PlayerContract |
||
294 | |||
295 | /** |
||
296 | 21 | * @param PlayerContract $player |
|
297 | */ |
||
298 | 21 | public function postSmallBlind(PlayerContract $player) |
|
308 | 4 | ||
309 | /** |
||
310 | 4 | * @param PlayerContract $player |
|
311 | 1 | */ |
|
312 | public function postBigBlind(PlayerContract $player) |
||
322 | |||
323 | 34 | /** |
|
324 | * @return Chips |
||
325 | 34 | */ |
|
326 | private function smallBlind(): Chips |
||
330 | |||
331 | /** |
||
332 | * @return Chips |
||
333 | */ |
||
334 | 34 | private function bigBlind(): Chips |
|
338 | |||
339 | 34 | /** |
|
340 | * @return ChipPot |
||
341 | 34 | */ |
|
342 | 34 | public function currentPot(): ChipPot |
|
346 | |||
347 | /** |
||
348 | 34 | * @return ChipPotCollection |
|
349 | */ |
||
350 | 34 | public function chipPots(): ChipPotCollection |
|
354 | |||
355 | /** |
||
356 | 34 | * @param PlayerContract $player |
|
357 | * |
||
358 | 34 | * @return Chips |
|
359 | */ |
||
360 | public function playerBetStack(PlayerContract $player): Chips |
||
364 | 14 | ||
365 | /** |
||
366 | 14 | * @param PlayerContract $player |
|
367 | * @param Chips $chips |
||
368 | */ |
||
369 | private function postBlind(PlayerContract $player, $chips) |
||
376 | |||
377 | /** |
||
378 | * @return PlayerContract|false |
||
379 | */ |
||
380 | public function whosTurnIsIt() |
||
406 | 2 | ||
407 | /** |
||
408 | 2 | * @return ChipPotCollection |
|
409 | 1 | */ |
|
410 | public function collectChipTotal(): ChipPotCollection |
||
491 | |||
492 | /** |
||
493 | 5 | * Deal the Flop. |
|
494 | */ |
||
495 | 5 | public function dealFlop() |
|
511 | |||
512 | /** |
||
513 | * Deal the turn card. |
||
514 | */ |
||
515 | 17 | public function dealTurn() |
|
531 | |||
532 | /** |
||
533 | 16 | * Deal the river card. |
|
534 | */ |
||
535 | public function dealRiver() |
||
551 | |||
552 | /** |
||
553 | 12 | * @throws RoundException |
|
554 | 12 | */ |
|
555 | public function checkPlayerTryingToAct(PlayerContract $player) |
||
565 | 1 | ||
566 | /** |
||
567 | * @param PlayerContract $player |
||
568 | 9 | * |
|
569 | 9 | * @throws RoundException |
|
570 | */ |
||
571 | public function playerCalls(PlayerContract $player) |
||
590 | |||
591 | /** |
||
592 | * @param PlayerContract $player |
||
593 | * @param Chips $chips |
||
594 | 33 | * |
|
595 | * @throws RoundException |
||
596 | 33 | */ |
|
597 | 33 | public function playerRaises(PlayerContract $player, Chips $chips) |
|
616 | |||
617 | 22 | /** |
|
618 | * @param PlayerContract $player |
||
619 | 22 | * |
|
620 | * @throws RoundException |
||
621 | 22 | */ |
|
622 | 22 | public function playerFoldsHand(PlayerContract $player) |
|
631 | 4 | ||
632 | /** |
||
633 | 4 | * @param PlayerContract $player |
|
634 | * |
||
635 | 4 | * @throws RoundException |
|
636 | */ |
||
637 | 4 | public function playerPushesAllIn(PlayerContract $player) |
|
650 | 13 | ||
651 | /** |
||
652 | 13 | * @param PlayerContract $player |
|
653 | 13 | * |
|
654 | 13 | * @throws RoundException |
|
655 | */ |
||
656 | public function playerChecks(PlayerContract $player) |
||
667 | |||
668 | /** |
||
669 | 14 | * @return Chips |
|
670 | */ |
||
671 | 14 | private function highestBet(): Chips |
|
677 | |||
678 | /** |
||
679 | * @param PlayerContract $player |
||
680 | 16 | * @param Chips $chips |
|
681 | */ |
||
682 | 16 | private function placeChipBet(PlayerContract $player, Chips $chips) |
|
694 | 22 | ||
695 | 22 | /** |
|
696 | * Reset the chip stack for all players. |
||
697 | */ |
||
698 | private function resetBetStacks() |
||
704 | 30 | ||
705 | 1 | /** |
|
706 | * Reset the leftToAct collection. |
||
707 | */ |
||
708 | private function setupLeftToAct() |
||
720 | 50 | ||
721 | 50 | /** |
|
722 | 50 | * @param PlayerContract $player |
|
723 | 50 | */ |
|
724 | public function sitPlayerOut(PlayerContract $player) |
||
729 | |||
730 | 50 | /** |
|
731 | 4 | * @var int |
|
732 | */ |
||
733 | 4 | public function resetPlayerList(int $seat) |
|
740 | } |
||
741 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the interface: