Skip to content

Code review exercise: Kevin Thompson #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kevint98 wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Code review exercise: Kevin Thompson #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
dee61f5
change add function to addPlayer
Kevin-aps Jan 8, 2024
115e31a
change howManyPlayers to getPlayerCount
Kevin-aps Jan 8, 2024
be16a46
change roll to rollDice
Kevin-aps Jan 8, 2024
725ff5e
change currentCategory to getCurrentCategory
Kevin-aps Jan 8, 2024
000f47a
change wasWrongAnswer to wasNotCorrectlyAnswered
Kevin-aps Jan 8, 2024
812f128
create nextPlayer method
Kevin-aps Jan 8, 2024
4d3cf6d
extract getAnswerOutcome to print outcome of answering question
Kevin-aps Jan 8, 2024
5a3e8b0
extract movePlaces to calculate new player location and ask question
Kevin-aps Jan 8, 2024
4e6dd2c
set constant to set max number of questions per category
Kevin-aps Jan 8, 2024
aaeda05
rename index variable
Kevin-aps Jan 8, 2024
ba01637
make MAX_QUESTIONS_PER_CATEGORY private
Kevin-aps Jan 9, 2024
eb86bcb
add game board and refactor getCurrentCategory
Kevin-aps Jan 9, 2024
7fd09ff
extract odd roll into function isRollOdd
Kevin-aps Jan 9, 2024
a1d7e14
rename isRollOdd to isRollOddAndDoesOtherThings
Kevin-aps Jan 9, 2024
02deb70
create GameBoard class and extract getCurrentCategory
Kevin-aps Jan 9, 2024
de3db5d
refactor movePlaces and create currentPlayerPlace
Kevin-aps Jan 9, 2024
7a4c647
refactor askQuestion
Kevin-aps Jan 9, 2024
c9c98fc
extract question functionality into GameBoard class
Kevin-aps Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
238 changes: 116 additions & 122 deletions exercises/php/trivia/Game.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,137 +2,85 @@

namespace trivia;

function echoln($string) {
echo $string."\n";
function echoln($string)
{
echo $string . "\n";
}

class Game {
var $players;
var $places;
var $purses ;
var $inPenaltyBox ;

var $popQuestions;
var $scienceQuestions;
var $sportsQuestions;
var $rockQuestions;

var $currentPlayer = 0;
var $isGettingOutOfPenaltyBox;

function __construct(){

$this->players = array();
$this->places = array(0);
$this->purses = array(0);
$this->inPenaltyBox = array(0);

$this->popQuestions = array();
$this->scienceQuestions = array();
$this->sportsQuestions = array();
$this->rockQuestions = array();

for ($i = 0; $i < 50; $i++) {
array_push($this->popQuestions, "Pop Question " . $i);
array_push($this->scienceQuestions, ("Science Question " . $i));
array_push($this->sportsQuestions, ("Sports Question " . $i));
array_push($this->rockQuestions, $this->createRockQuestion($i));
}
}

function createRockQuestion($index){
class Game
{
var $players;
var $places;
var $board;
var $purses;
var $inPenaltyBox;

var $popQuestions;
var $scienceQuestions;
var $sportsQuestions;
var $rockQuestions;

var $currentPlayer = 0;
var $isGettingOutOfPenaltyBox;

function __construct()
{
$this->board = new GameBoard();


$this->players = array();
$this->places = array(0);
$this->purses = array(0);
$this->inPenaltyBox = array(0);
}

function createRockQuestion($index)
{
return "Rock Question " . $index;
}

function isPlayable() {
return ($this->howManyPlayers() >= 2);
function isPlayable()
{
return ($this->getPlayerCount() >= 2);
}

function add($playerName) {
array_push($this->players, $playerName);
$this->places[$this->howManyPlayers()] = 0;
$this->purses[$this->howManyPlayers()] = 0;
$this->inPenaltyBox[$this->howManyPlayers()] = false;
function addPlayer($playerName)
{
array_push($this->players, $playerName);
$this->places[$this->getPlayerCount()] = 0;
$this->purses[$this->getPlayerCount()] = 0;
$this->inPenaltyBox[$this->getPlayerCount()] = false;

echoln($playerName . " was added");
echoln("They are player number " . count($this->players));
echoln($playerName . " was added");
echoln("They are player number " . count($this->players));
return true;
}

function howManyPlayers() {
function getPlayerCount()
{
return count($this->players);
}

function roll($roll) {
function rollDice($roll)
{
echoln($this->players[$this->currentPlayer] . " is the current player");
echoln("They have rolled a " . $roll);

if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($roll % 2 != 0) {
$this->isGettingOutOfPenaltyBox = true;

echoln($this->players[$this->currentPlayer] . " is getting out of the penalty box");
$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $roll;
if ($this->places[$this->currentPlayer] > 11) $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;

echoln($this->players[$this->currentPlayer]
. "'s new location is "
.$this->places[$this->currentPlayer]);
echoln("The category is " . $this->currentCategory());
$this->askQuestion();
} else {
echoln($this->players[$this->currentPlayer] . " is not getting out of the penalty box");
$this->isGettingOutOfPenaltyBox = false;
}

$this->isRollOddAndDoesOtherThings($roll);
} else {

$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $roll;
if ($this->places[$this->currentPlayer] > 11) $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;

echoln($this->players[$this->currentPlayer]
. "'s new location is "
.$this->places[$this->currentPlayer]);
echoln("The category is " . $this->currentCategory());
$this->askQuestion();
$this->movePlaces($roll);
}

}

function askQuestion() {
if ($this->currentCategory() == "Pop")
echoln(array_shift($this->popQuestions));
if ($this->currentCategory() == "Science")
echoln(array_shift($this->scienceQuestions));
if ($this->currentCategory() == "Sports")
echoln(array_shift($this->sportsQuestions));
if ($this->currentCategory() == "Rock")
echoln(array_shift($this->rockQuestions));
}


function currentCategory() {
if ($this->places[$this->currentPlayer] == 0) return "Pop";
if ($this->places[$this->currentPlayer] == 4) return "Pop";
if ($this->places[$this->currentPlayer] == 8) return "Pop";
if ($this->places[$this->currentPlayer] == 1) return "Science";
if ($this->places[$this->currentPlayer] == 5) return "Science";
if ($this->places[$this->currentPlayer] == 9) return "Science";
if ($this->places[$this->currentPlayer] == 2) return "Sports";
if ($this->places[$this->currentPlayer] == 6) return "Sports";
if ($this->places[$this->currentPlayer] == 10) return "Sports";
return "Rock";
}

function wasCorrectlyAnswered() {
if ($this->inPenaltyBox[$this->currentPlayer]){
function wasCorrectlyAnswered()
{
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isGettingOutOfPenaltyBox) {
echoln("Answer was correct!!!!");
$this->purses[$this->currentPlayer]++;
echoln($this->players[$this->currentPlayer]
. " now has "
.$this->purses[$this->currentPlayer]
. " Gold Coins.");
$this->getAnswerOutcome('correct');

$winner = $this->didPlayerWin();
$this->currentPlayer++;
Expand All @@ -144,38 +92,84 @@ function wasCorrectlyAnswered() {
if ($this->currentPlayer == count($this->players)) $this->currentPlayer = 0;
Copy link

@smpezert smpezert Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it possible to avoid this magic number and add it somewhere else?

return true;
}



} else {

echoln("Answer was corrent!!!!");
$this->purses[$this->currentPlayer]++;
echoln($this->players[$this->currentPlayer]
. " now has "
.$this->purses[$this->currentPlayer]
. " Gold Coins.");
$this->getAnswerOutcome('correct');
Copy link

@smpezert smpezert Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could probably extract this and add it to a variable on the top called "correct" or "correctAnswer".


$winner = $this->didPlayerWin();
$this->currentPlayer++;
if ($this->currentPlayer == count($this->players)) $this->currentPlayer = 0;

$this->nextPlayer();

return $winner;
}
}

function wrongAnswer(){
echoln("Question was incorrectly answered");
echoln($this->players[$this->currentPlayer] . " was sent to the penalty box");
$this->inPenaltyBox[$this->currentPlayer] = true;
function wasNotCorrectlyAnswered()
{
$this->getAnswerOutcome('incorrect');
Copy link

@smpezert smpezert Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here.


$this->inPenaltyBox[$this->currentPlayer] = true;

$this->nextPlayer();

$this->currentPlayer++;
if ($this->currentPlayer == count($this->players)) $this->currentPlayer = 0;
return true;
}


function didPlayerWin() {
function didPlayerWin()
{
return !($this->purses[$this->currentPlayer] == 6);
Copy link

@smpezert smpezert Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see a magic number that you could probably extract it.

}

function nextPlayer()
{
$this->currentPlayer++;
if ($this->currentPlayer == count($this->players)) $this->currentPlayer = 0;
}

function getAnswerOutcome($answer)
{
if ($answer === "correct") {
$this->purses[$this->currentPlayer]++;
echoln($this->players[$this->currentPlayer]
. " now has "
. $this->purses[$this->currentPlayer]
. " Gold Coins.");
} else {
echoln("Question was incorrectly answered");
echoln($this->players[$this->currentPlayer] . " was sent to the penalty box");
}
}

function movePlaces($places)
{
$this->places[$this->currentPlayer] = $this->currentPlayerPlace() + $places;
if ($this->places[$this->currentPlayer] > 11) $this->places[$this->currentPlayer] = $this->currentPlayerPlace() - 12;

echoln($this->players[$this->currentPlayer]
. "'s new location is "
. $this->currentPlayerPlace());
echoln("The category is " . $this->board->getcurrentCategory($this->currentPlayerPlace()));

$this->board->askQuestion($this->currentPlayerPlace());
}

function currentPlayerPlace()
{
return $this->places[$this->currentPlayer];
}

function isRollOddAndDoesOtherThings($roll)
{
if ($roll % 2 != 0) {
$this->isGettingOutOfPenaltyBox = true;

echoln($this->players[$this->currentPlayer] . " is getting out of the penalty box");
$this->movePlaces($roll);
} else {
echoln($this->players[$this->currentPlayer] . " is not getting out of the penalty box");
$this->isGettingOutOfPenaltyBox = false;
}
}
}
59 changes: 59 additions & 0 deletions exercises/php/trivia/GameBoard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

namespace trivia;

class GameBoard
{
private $places;
private $popQuestions;
private $scienceQuestions;
private $sportsQuestions;
private $rockQuestions;

private const MAX_QUESTIONS_PER_CATEGORY = 50;

function __construct()
{
$this->places = [
"Pop", "Science", "Sports",
"Rock", "Pop", "Science",
"Sports", "Rock", "Pop",
"Science", "Sports", "Rock"
];

$this->popQuestions = array();
$this->scienceQuestions = array();
$this->sportsQuestions = array();
$this->rockQuestions = array();

for ($index = 0; $index < self::MAX_QUESTIONS_PER_CATEGORY; $index++) {
array_push($this->popQuestions, "Pop Question " . $index);
array_push($this->scienceQuestions, ("Science Question " . $index));
array_push($this->sportsQuestions, ("Sports Question " . $index));
array_push($this->rockQuestions, $this->createRockQuestion($index));
}
}

function createRockQuestion($index)
{
return "Rock Question " . $index;
}


function getcurrentCategory($currentPlayerPosition)
{
return $this->places[$currentPlayerPosition];
}

function askQuestion($currentPlayerPosition)
{
if ($this->getcurrentCategory($currentPlayerPosition) == "Pop")
echoln(array_shift($this->popQuestions));
if ($this->getcurrentCategory($currentPlayerPosition) == "Science")
echoln(array_shift($this->scienceQuestions));
if ($this->getcurrentCategory($currentPlayerPosition) == "Sports")
echoln(array_shift($this->sportsQuestions));;
if ($this->getcurrentCategory($currentPlayerPosition) == "Rock")
echoln(array_shift($this->rockQuestions));
}
}
10 changes: 5 additions & 5 deletions exercises/php/trivia/GameRunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ public function runGame($seed): void

$aGame = new Game();

$aGame->add("Chet");
$aGame->add("Pat");
$aGame->add("Sue");
$aGame->addPlayer("Chet");
$aGame->addPlayer("Pat");
$aGame->addPlayer("Sue");

srand($seed);
do {

$aGame->roll(rand(0,5) + 1);
$aGame->rollDice(rand(0,5) + 1);

if (rand(0,9) == 7) {
$notAWinner = $aGame->wrongAnswer();
$notAWinner = $aGame->wasNotCorrectlyAnswered();
} else {
$notAWinner = $aGame->wasCorrectlyAnswered();
}
Expand Down