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() |
|
205 | { |
||
206 | $this->chipPots() |
||
207 | ->reverse() |
||
208 | ->each(function (ChipPot $chipPot) { |
||
209 | // if only 1 player participated to pot, he wins it no arguments |
||
210 | 22 | if ($chipPot->players()->count() === 1) { |
|
211 | $potTotal = $chipPot->chips()->total(); |
||
212 | 22 | ||
213 | 22 | $chipPot->players()->first()->chipStack()->add($potTotal); |
|
214 | |||
215 | $this->chipPots()->remove($chipPot); |
||
216 | |||
217 | return; |
||
218 | 2 | } |
|
219 | |||
220 | 2 | $activePlayers = $chipPot->players()->diff($this->foldedPlayers()); |
|
221 | |||
222 | $playerHands = $this->dealer()->hands()->findByPlayers($activePlayers); |
||
223 | $evaluate = $this->dealer()->evaluateHands($this->dealer()->communityCards(), $playerHands); |
||
224 | |||
225 | // if just 1, the player with that hand wins |
||
226 | 11 | if ($evaluate->count() === 1) { |
|
227 | $player = $evaluate->first()->hand()->player(); |
||
228 | 11 | $potTotal = $chipPot->chips()->total(); |
|
229 | 11 | ||
230 | $player->chipStack()->add($potTotal); |
||
231 | |||
232 | 11 | $this->chipPots()->remove($chipPot); |
|
233 | 6 | } else { |
|
234 | // if > 1 hand is evaluated as highest, split the pot evenly between the players |
||
235 | 6 | ||
236 | $potTotal = $chipPot->chips()->total(); |
||
237 | 6 | ||
238 | // split the pot between the number of players |
||
239 | 6 | $splitTotal = Chips::fromAmount(($potTotal->amount() / $evaluate->count())); |
|
240 | $evaluate->each(function (CardResults $result) use ($splitTotal) { |
||
241 | $result->hand()->player()->chipStack()->add($splitTotal); |
||
242 | 11 | }); |
|
243 | |||
244 | 11 | $this->chipPots()->remove($chipPot); |
|
245 | 11 | } |
|
246 | }) |
||
247 | ; |
||
248 | 11 | } |
|
249 | 11 | ||
250 | 11 | /** |
|
251 | * @param Player $actualPlayer |
||
252 | 11 | * |
|
253 | * @return bool |
||
254 | 11 | */ |
|
255 | public function playerIsStillIn(PlayerContract $actualPlayer) |
||
261 | |||
262 | 1 | /** |
|
263 | * @return PlayerContract |
||
264 | 1 | */ |
|
265 | 1 | public function playerWithButton(): PlayerContract |
|
269 | 11 | ||
270 | /** |
||
271 | 11 | * @return PlayerContract |
|
272 | */ |
||
273 | public function playerWithSmallBlind(): PlayerContract |
||
281 | |||
282 | 1 | /** |
|
283 | * @return PlayerContract |
||
284 | */ |
||
285 | public function playerWithBigBlind(): PlayerContract |
||
293 | |||
294 | /** |
||
295 | * @param PlayerContract $player |
||
296 | 21 | */ |
|
297 | public function postSmallBlind(PlayerContract $player) |
||
307 | |||
308 | 4 | /** |
|
309 | * @param PlayerContract $player |
||
310 | 4 | */ |
|
311 | 1 | public function postBigBlind(PlayerContract $player) |
|
321 | |||
322 | /** |
||
323 | 34 | * @return Chips |
|
324 | */ |
||
325 | 34 | private function smallBlind(): Chips |
|
329 | 34 | ||
330 | /** |
||
331 | * @return Chips |
||
332 | */ |
||
333 | private function bigBlind(): Chips |
||
337 | 34 | ||
338 | /** |
||
339 | 34 | * @return ChipPot |
|
340 | */ |
||
341 | 34 | public function currentPot(): ChipPot |
|
345 | |||
346 | /** |
||
347 | * @return ChipPotCollection |
||
348 | 34 | */ |
|
349 | public function chipPots(): ChipPotCollection |
||
353 | |||
354 | /** |
||
355 | * @param PlayerContract $player |
||
356 | 34 | * |
|
357 | * @return Chips |
||
358 | 34 | */ |
|
359 | public function playerBetStack(PlayerContract $player): Chips |
||
363 | |||
364 | 14 | /** |
|
365 | * @param PlayerContract $player |
||
366 | 14 | * @param Chips $chips |
|
367 | */ |
||
368 | private function postBlind(PlayerContract $player, $chips) |
||
375 | |||
376 | /** |
||
377 | * @return PlayerContract|false |
||
378 | */ |
||
379 | public function whosTurnIsIt() |
||
393 | 34 | ||
394 | /** |
||
395 | * @return ChipPotCollection |
||
396 | 34 | */ |
|
397 | 34 | public function collectChipTotal(): ChipPotCollection |
|
477 | 6 | ||
478 | 6 | /** |
|
479 | 6 | * Deal the Flop. |
|
480 | */ |
||
481 | public function dealFlop() |
||
497 | 5 | ||
498 | 5 | /** |
|
499 | 5 | * Deal the turn card. |
|
500 | 6 | */ |
|
501 | public function dealTurn() |
||
517 | 17 | ||
518 | 1 | /** |
|
519 | * Deal the river card. |
||
520 | 17 | */ |
|
521 | 1 | public function dealRiver() |
|
537 | 16 | ||
538 | 16 | /** |
|
539 | 16 | * @throws RoundException |
|
540 | */ |
||
541 | public function checkPlayerTryingToAct(PlayerContract $player) |
||
551 | |||
552 | /** |
||
553 | 12 | * @param PlayerContract $player |
|
554 | 12 | * |
|
555 | * @throws RoundException |
||
556 | */ |
||
557 | public function playerCalls(PlayerContract $player) |
||
576 | 12 | ||
577 | /** |
||
578 | 12 | * @param PlayerContract $player |
|
579 | 12 | * @param Chips $chips |
|
580 | 12 | * |
|
581 | 12 | * @throws RoundException |
|
582 | 12 | */ |
|
583 | public function playerRaises(PlayerContract $player, Chips $chips) |
||
602 | |||
603 | 31 | /** |
|
604 | * @param PlayerContract $player |
||
605 | * |
||
606 | * @throws RoundException |
||
607 | */ |
||
608 | public function playerFoldsHand(PlayerContract $player) |
||
617 | 22 | ||
618 | /** |
||
619 | 22 | * @param PlayerContract $player |
|
620 | * |
||
621 | 22 | * @throws RoundException |
|
622 | 22 | */ |
|
623 | 22 | public function playerPushesAllIn(PlayerContract $player) |
|
636 | |||
637 | 4 | /** |
|
638 | 3 | * @param PlayerContract $player |
|
639 | 3 | * |
|
640 | * @throws RoundException |
||
641 | */ |
||
642 | public function playerChecks(PlayerContract $player) |
||
653 | 13 | ||
654 | 13 | /** |
|
655 | * @return Chips |
||
656 | */ |
||
657 | private function highestBet(): Chips |
||
663 | 14 | ||
664 | /** |
||
665 | * @param PlayerContract $player |
||
666 | 14 | * @param Chips $chips |
|
667 | */ |
||
668 | private function placeChipBet(PlayerContract $player, Chips $chips) |
||
680 | 16 | ||
681 | /** |
||
682 | 16 | * Reset the chip stack for all players. |
|
683 | */ |
||
684 | 15 | private function resetBetStacks() |
|
690 | |||
691 | 22 | /** |
|
692 | * Reset the leftToAct collection. |
||
693 | */ |
||
694 | 22 | private function setupLeftToAct() |
|
706 | |||
707 | /** |
||
708 | * @param PlayerContract $player |
||
709 | 30 | */ |
|
710 | public function sitPlayerOut(PlayerContract $player) |
||
715 | |||
716 | /** |
||
717 | * @var int |
||
718 | */ |
||
719 | public function resetPlayerList(int $seat) |
||
726 | } |
||
727 |
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: