From 0c3f14c9127e2843814356c9424b583de74a9170 Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 15:43:35 +0200 Subject: [PATCH 01/10] Rename classes, interfaces, attributes and variables in src/ in order to provide Persons semantics --- src/Algorithm/F.php | 17 ------ src/Algorithm/FT.php | 11 ---- src/Algorithm/Finder.php | 70 ++++++++++++++----------- src/Algorithm/FinderCriteria.php | 11 ++++ src/Algorithm/{Thing.php => Person.php} | 2 +- src/Algorithm/PersonsPair.php | 17 ++++++ tests/Algorithm/FinderTest.php | 56 ++++++++++---------- 7 files changed, 97 insertions(+), 87 deletions(-) delete mode 100644 src/Algorithm/F.php delete mode 100644 src/Algorithm/FT.php create mode 100644 src/Algorithm/FinderCriteria.php rename src/Algorithm/{Thing.php => Person.php} (96%) create mode 100644 src/Algorithm/PersonsPair.php diff --git a/src/Algorithm/F.php b/src/Algorithm/F.php deleted file mode 100644 index 3db7792..0000000 --- a/src/Algorithm/F.php +++ /dev/null @@ -1,17 +0,0 @@ -_p = $p; + $this->allPersons = $allPersons; } - public function find(int $ft): F + public function find(int $finderCriteria): PersonsPair { - /** @var F[] $tr */ - $tr = []; + /** @var PersonsPair[] $allPersonsPairs */ + $allPersonsPairs = []; - for ($i = 0; $i < count($this->_p); $i++) { - for ($j = $i + 1; $j < count($this->_p); $j++) { - $r = new F(); + $numberOfPersons = count($this->allPersons); - if ($this->_p[$i]->birthDate < $this->_p[$j]->birthDate) { - $r->p1 = $this->_p[$i]; - $r->p2 = $this->_p[$j]; + for ($i = 0; $i < $numberOfPersons; $i++) { + for ($j = $i + 1; $j < $numberOfPersons; $j++) { + $personsPair = new PersonsPair(); + + if ($this->allPersons[$i]->birthDate + < $this->allPersons[$j]->birthDate + ) { + $personsPair->person1 = $this->allPersons[$i]; + $personsPair->person2 = $this->allPersons[$j]; } else { - $r->p1 = $this->_p[$j]; - $r->p2 = $this->_p[$i]; + $personsPair->person1 = $this->allPersons[$j]; + $personsPair->person2 = $this->allPersons[$i]; } - $r->d = $r->p2->birthDate->getTimestamp() - - $r->p1->birthDate->getTimestamp(); + $personsPair->birthDaysDistanceInSeconds = + $personsPair->person2->birthDate->getTimestamp() + - $personsPair->person1->birthDate->getTimestamp(); - $tr[] = $r; + $allPersonsPairs[] = $personsPair; } } - if (count($tr) < 1) { - return new F(); + $thereAreNoPersonsPairs = count($allPersonsPairs) < 1; + if ($thereAreNoPersonsPairs) { + return new PersonsPair(); } - $answer = $tr[0]; + $personsPairMatchingCriteria = $allPersonsPairs[0]; - foreach ($tr as $result) { - switch ($ft) { - case FT::ONE: - if ($result->d < $answer->d) { - $answer = $result; + foreach ($allPersonsPairs as $personsPair) { + switch ($finderCriteria) { + case FinderCriteria::CLOSEST_BIRTHDAYS: + if ($personsPair->birthDaysDistanceInSeconds + < $personsPairMatchingCriteria->birthDaysDistanceInSeconds + ) { + $personsPairMatchingCriteria = $personsPair; } break; - case FT::TWO: - if ($result->d > $answer->d) { - $answer = $result; + case FinderCriteria::FURTHEST_BIRTHDAYS: + if ($personsPair->birthDaysDistanceInSeconds + > $personsPairMatchingCriteria->birthDaysDistanceInSeconds + ) { + $personsPairMatchingCriteria = $personsPair; } break; } } - return $answer; + return $personsPairMatchingCriteria; } } diff --git a/src/Algorithm/FinderCriteria.php b/src/Algorithm/FinderCriteria.php new file mode 100644 index 0000000..0fc7f8b --- /dev/null +++ b/src/Algorithm/FinderCriteria.php @@ -0,0 +1,11 @@ +sue = new Thing(); + $this->sue = new Person(); $this->sue->name = "Sue"; $this->sue->birthDate = new \DateTime("1950-01-01"); - $this->greg = new Thing(); + $this->greg = new Person(); $this->greg->name = "Greg"; $this->greg->birthDate = new \DateTime("1952-05-01"); - $this->sarah = new Thing(); + $this->sarah = new Person(); $this->sarah->name = "Sarah"; $this->sarah->birthDate = new \DateTime("1982-01-01"); - $this->mike = new Thing(); + $this->mike = new Person(); $this->mike->name = "Mike"; $this->mike->birthDate = new \DateTime("1979-01-01"); } @@ -48,10 +48,10 @@ public function should_return_empty_when_given_empty_list() $list = []; $finder = new Finder($list); - $result = $finder->find(FT::ONE); + $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); - $this->assertEquals(null, $result->p1); - $this->assertEquals(null, $result->p2); + $this->assertEquals(null, $result->person1); + $this->assertEquals(null, $result->person2); } /** @test */ @@ -61,10 +61,10 @@ public function should_return_empty_when_given_one_person() $list[] = $this->sue; $finder = new Finder($list); - $result = $finder->find(FT::ONE); + $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); - $this->assertEquals(null, $result->p1); - $this->assertEquals(null, $result->p2); + $this->assertEquals(null, $result->person1); + $this->assertEquals(null, $result->person2); } /** @test */ @@ -75,10 +75,10 @@ public function should_return_closest_two_for_two_people() $list[] = $this->greg; $finder = new Finder($list); - $result = $finder->find(FT::ONE); + $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); - $this->assertEquals($this->sue, $result->p1); - $this->assertEquals($this->greg, $result->p2); + $this->assertEquals($this->sue, $result->person1); + $this->assertEquals($this->greg, $result->person2); } /** @test */ @@ -89,10 +89,10 @@ public function should_return_furthest_two_for_two_people() $list[] = $this->greg; $finder = new Finder($list); - $result = $finder->find(FT::TWO); + $result = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); - $this->assertEquals($this->greg, $result->p1); - $this->assertEquals($this->mike, $result->p2); + $this->assertEquals($this->greg, $result->person1); + $this->assertEquals($this->mike, $result->person2); } /** @test */ @@ -105,10 +105,10 @@ public function should_return_furthest_two_for_four_people() $list[] = $this->greg; $finder = new Finder($list); - $result = $finder->find(FT::TWO); + $result = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); - $this->assertEquals($this->sue, $result->p1); - $this->assertEquals($this->sarah, $result->p2); + $this->assertEquals($this->sue, $result->person1); + $this->assertEquals($this->sarah, $result->person2); } /** @@ -123,9 +123,9 @@ public function should_return_closest_two_for_four_people() $list[] = $this->greg; $finder = new Finder($list); - $result = $finder->find(FT::ONE); + $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); - $this->assertEquals($this->sue, $result->p1); - $this->assertEquals($this->greg, $result->p2); + $this->assertEquals($this->sue, $result->person1); + $this->assertEquals($this->greg, $result->person2); } } From 99b77f946ad3091320e5c774d1ee4e1751ceef34 Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 15:48:19 +0200 Subject: [PATCH 02/10] Rename variables in tests/ in order to provide Persons semantics --- tests/Algorithm/FinderTest.php | 41 +++++++++++----------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/tests/Algorithm/FinderTest.php b/tests/Algorithm/FinderTest.php index b8391e0..391b7a9 100644 --- a/tests/Algorithm/FinderTest.php +++ b/tests/Algorithm/FinderTest.php @@ -45,8 +45,8 @@ protected function setUp() /** @test */ public function should_return_empty_when_given_empty_list() { - $list = []; - $finder = new Finder($list); + $allPersons = []; + $finder = new Finder($allPersons); $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); @@ -57,9 +57,8 @@ public function should_return_empty_when_given_empty_list() /** @test */ public function should_return_empty_when_given_one_person() { - $list = []; - $list[] = $this->sue; - $finder = new Finder($list); + $allPersons = [$this->sue]; + $finder = new Finder($allPersons); $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); @@ -70,10 +69,8 @@ public function should_return_empty_when_given_one_person() /** @test */ public function should_return_closest_two_for_two_people() { - $list = []; - $list[] = $this->sue; - $list[] = $this->greg; - $finder = new Finder($list); + $allPersons = [$this->sue, $this->greg]; + $finder = new Finder($allPersons); $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); @@ -84,10 +81,8 @@ public function should_return_closest_two_for_two_people() /** @test */ public function should_return_furthest_two_for_two_people() { - $list = []; - $list[] = $this->mike; - $list[] = $this->greg; - $finder = new Finder($list); + $allPersons = [$this->mike, $this->greg]; + $finder = new Finder($allPersons); $result = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); @@ -98,12 +93,8 @@ public function should_return_furthest_two_for_two_people() /** @test */ public function should_return_furthest_two_for_four_people() { - $list = []; - $list[] = $this->sue; - $list[] = $this->sarah; - $list[] = $this->mike; - $list[] = $this->greg; - $finder = new Finder($list); + $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; + $finder = new Finder($allPersons); $result = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); @@ -111,17 +102,11 @@ public function should_return_furthest_two_for_four_people() $this->assertEquals($this->sarah, $result->person2); } - /** - * @test - */ + /** @test */ public function should_return_closest_two_for_four_people() { - $list = []; - $list[] = $this->sue; - $list[] = $this->sarah; - $list[] = $this->mike; - $list[] = $this->greg; - $finder = new Finder($list); + $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; + $finder = new Finder($allPersons); $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); From ad0aa8adb69acf5fed53c51f3fe4e037b6301d09 Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 16:09:14 +0200 Subject: [PATCH 03/10] * Make `Person` and `PersonsPair` attributes private * Create constructor for `Person` and `PersonsPair` classes and remove setters * Remove the get prefix from getters * Move the responsibility of calculating the `birthDaysDistanceInSeconds` from the `Finder` to the `PersonsPair` * Throw an exception `NotEnoughPersonsException` in case of receiving not enough persons in order to find a pair. * Refactor the test in order to expect the new exception instead of an empty/inconsistent `PersonsPair` --- src/Algorithm/Finder.php | 31 ++++++----- src/Algorithm/NotEnoughPersonsException.php | 8 +++ src/Algorithm/Person.php | 20 +++---- src/Algorithm/PersonsPair.php | 33 ++++++++++-- tests/Algorithm/FinderTest.php | 59 +++++++++------------ 5 files changed, 85 insertions(+), 66 deletions(-) create mode 100644 src/Algorithm/NotEnoughPersonsException.php diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index 1746284..6a582ab 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -23,29 +23,26 @@ public function find(int $finderCriteria): PersonsPair for ($i = 0; $i < $numberOfPersons; $i++) { for ($j = $i + 1; $j < $numberOfPersons; $j++) { - $personsPair = new PersonsPair(); - if ($this->allPersons[$i]->birthDate - < $this->allPersons[$j]->birthDate + if ($this->allPersons[$i]->birthDate() + < $this->allPersons[$j]->birthDate() ) { - $personsPair->person1 = $this->allPersons[$i]; - $personsPair->person2 = $this->allPersons[$j]; + $personsPair = new PersonsPair( + $this->allPersons[$i], $this->allPersons[$j] + ); } else { - $personsPair->person1 = $this->allPersons[$j]; - $personsPair->person2 = $this->allPersons[$i]; + $personsPair = new PersonsPair( + $this->allPersons[$j], $this->allPersons[$i] + ); } - $personsPair->birthDaysDistanceInSeconds = - $personsPair->person2->birthDate->getTimestamp() - - $personsPair->person1->birthDate->getTimestamp(); - $allPersonsPairs[] = $personsPair; } } $thereAreNoPersonsPairs = count($allPersonsPairs) < 1; if ($thereAreNoPersonsPairs) { - return new PersonsPair(); + throw new NotEnoughPersonsException(); } $personsPairMatchingCriteria = $allPersonsPairs[0]; @@ -53,16 +50,18 @@ public function find(int $finderCriteria): PersonsPair foreach ($allPersonsPairs as $personsPair) { switch ($finderCriteria) { case FinderCriteria::CLOSEST_BIRTHDAYS: - if ($personsPair->birthDaysDistanceInSeconds - < $personsPairMatchingCriteria->birthDaysDistanceInSeconds + if ($personsPair->birthDaysDistanceInSeconds() + < $personsPairMatchingCriteria->birthDaysDistanceInSeconds( + ) ) { $personsPairMatchingCriteria = $personsPair; } break; case FinderCriteria::FURTHEST_BIRTHDAYS: - if ($personsPair->birthDaysDistanceInSeconds - > $personsPairMatchingCriteria->birthDaysDistanceInSeconds + if ($personsPair->birthDaysDistanceInSeconds() + > $personsPairMatchingCriteria->birthDaysDistanceInSeconds( + ) ) { $personsPairMatchingCriteria = $personsPair; } diff --git a/src/Algorithm/NotEnoughPersonsException.php b/src/Algorithm/NotEnoughPersonsException.php new file mode 100644 index 0000000..22a6f0f --- /dev/null +++ b/src/Algorithm/NotEnoughPersonsException.php @@ -0,0 +1,8 @@ +name; + $this->name = $aName; + $this->birthDate = $aBirthDate; } - public function setName(string $name) + public function name(): string { - $this->name = $name; + return $this->name; } - public function getBirthDate(): DateTime + public function birthDate(): DateTime { return $this->birthDate; } - - public function setBirthDate(DateTime $birthDate) - { - $this->birthDate = $birthDate; - } } diff --git a/src/Algorithm/PersonsPair.php b/src/Algorithm/PersonsPair.php index 11c9fd2..6dc0674 100644 --- a/src/Algorithm/PersonsPair.php +++ b/src/Algorithm/PersonsPair.php @@ -7,11 +7,38 @@ final class PersonsPair { /** @var Person */ - public $person1; + private $person1; /** @var Person */ - public $person2; + private $person2; /** @var int */ - public $birthDaysDistanceInSeconds; + private $birthDaysDistanceInSeconds; + + public function __construct(Person $aPerson1, Person $aPerson2) + { + $this->person1 = $aPerson1; + $this->person2 = $aPerson2; + $this->birthDaysDistanceInSeconds = $this->calculateBirthDaysDistanceInSeconds(); + } + + public function person1(): Person + { + return $this->person1; + } + + public function person2(): Person + { + return $this->person2; + } + + public function birthDaysDistanceInSeconds(): int + { + return $this->birthDaysDistanceInSeconds; + } + + private function calculateBirthDaysDistanceInSeconds(): int + { + return $this->person2()->birthDate()->getTimestamp() - $this->person1()->birthDate()->getTimestamp(); + } } diff --git a/tests/Algorithm/FinderTest.php b/tests/Algorithm/FinderTest.php index 391b7a9..fc73248 100644 --- a/tests/Algorithm/FinderTest.php +++ b/tests/Algorithm/FinderTest.php @@ -6,6 +6,7 @@ use CodelyTV\FinderKata\Algorithm\Finder; use CodelyTV\FinderKata\Algorithm\FinderCriteria; +use CodelyTV\FinderKata\Algorithm\NotEnoughPersonsException; use CodelyTV\FinderKata\Algorithm\Person; use PHPUnit\Framework\TestCase; @@ -25,45 +26,33 @@ final class FinderTest extends TestCase protected function setUp() { - $this->sue = new Person(); - $this->sue->name = "Sue"; - $this->sue->birthDate = new \DateTime("1950-01-01"); - - $this->greg = new Person(); - $this->greg->name = "Greg"; - $this->greg->birthDate = new \DateTime("1952-05-01"); - - $this->sarah = new Person(); - $this->sarah->name = "Sarah"; - $this->sarah->birthDate = new \DateTime("1982-01-01"); - - $this->mike = new Person(); - $this->mike->name = "Mike"; - $this->mike->birthDate = new \DateTime("1979-01-01"); + $this->sue = new Person("Sue", new \DateTime("1950-01-01")); + $this->greg = new Person("Greg", new \DateTime("1952-05-01")); + $this->sarah = new Person("Sarah", new \DateTime("1982-01-01")); + $this->mike = new Person("Mike", new \DateTime("1979-01-01")); } /** @test */ - public function should_return_empty_when_given_empty_list() + public function should_throw_not_enough_persons_exception_when_given_empty_list( + ) { $allPersons = []; $finder = new Finder($allPersons); - $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $this->expectException(NotEnoughPersonsException::class); - $this->assertEquals(null, $result->person1); - $this->assertEquals(null, $result->person2); + $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); } /** @test */ - public function should_return_empty_when_given_one_person() + public function should_throw_not_enough_persons_when_given_one_person() { $allPersons = [$this->sue]; $finder = new Finder($allPersons); - $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $this->expectException(NotEnoughPersonsException::class); - $this->assertEquals(null, $result->person1); - $this->assertEquals(null, $result->person2); + $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); } /** @test */ @@ -72,10 +61,10 @@ public function should_return_closest_two_for_two_people() $allPersons = [$this->sue, $this->greg]; $finder = new Finder($allPersons); - $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); - $this->assertEquals($this->sue, $result->person1); - $this->assertEquals($this->greg, $result->person2); + $this->assertEquals($this->sue, $personsPairFound->person1()); + $this->assertEquals($this->greg, $personsPairFound->person2()); } /** @test */ @@ -84,10 +73,10 @@ public function should_return_furthest_two_for_two_people() $allPersons = [$this->mike, $this->greg]; $finder = new Finder($allPersons); - $result = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); - $this->assertEquals($this->greg, $result->person1); - $this->assertEquals($this->mike, $result->person2); + $this->assertEquals($this->greg, $personsPairFound->person1()); + $this->assertEquals($this->mike, $personsPairFound->person2()); } /** @test */ @@ -96,10 +85,10 @@ public function should_return_furthest_two_for_four_people() $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; $finder = new Finder($allPersons); - $result = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); - $this->assertEquals($this->sue, $result->person1); - $this->assertEquals($this->sarah, $result->person2); + $this->assertEquals($this->sue, $personsPairFound->person1()); + $this->assertEquals($this->sarah, $personsPairFound->person2()); } /** @test */ @@ -108,9 +97,9 @@ public function should_return_closest_two_for_four_people() $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; $finder = new Finder($allPersons); - $result = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); - $this->assertEquals($this->sue, $result->person1); - $this->assertEquals($this->greg, $result->person2); + $this->assertEquals($this->sue, $personsPairFound->person1()); + $this->assertEquals($this->greg, $personsPairFound->person2()); } } From d5397c6e262820cd898c3a72a262eeb215e5d919 Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 16:20:43 +0200 Subject: [PATCH 04/10] Moderate `FinderCriteria` as a Value Object instead of a plain `interface` with two `const` --- src/Algorithm/Finder.php | 32 ++++++++++------------- src/Algorithm/FinderCriteria.php | 44 +++++++++++++++++++++++++++++--- tests/Algorithm/FinderTest.php | 12 ++++----- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index 6a582ab..de9e4fa 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -14,7 +14,7 @@ public function __construct(array $allPersons) $this->allPersons = $allPersons; } - public function find(int $finderCriteria): PersonsPair + public function find(FinderCriteria $finderCriteria): PersonsPair { /** @var PersonsPair[] $allPersonsPairs */ $allPersonsPairs = []; @@ -48,24 +48,18 @@ public function find(int $finderCriteria): PersonsPair $personsPairMatchingCriteria = $allPersonsPairs[0]; foreach ($allPersonsPairs as $personsPair) { - switch ($finderCriteria) { - case FinderCriteria::CLOSEST_BIRTHDAYS: - if ($personsPair->birthDaysDistanceInSeconds() - < $personsPairMatchingCriteria->birthDaysDistanceInSeconds( - ) - ) { - $personsPairMatchingCriteria = $personsPair; - } - break; - - case FinderCriteria::FURTHEST_BIRTHDAYS: - if ($personsPair->birthDaysDistanceInSeconds() - > $personsPairMatchingCriteria->birthDaysDistanceInSeconds( - ) - ) { - $personsPairMatchingCriteria = $personsPair; - } - break; + if ($finderCriteria->isByClosestBirthDates()) { + if ($personsPair->birthDaysDistanceInSeconds() + < $personsPairMatchingCriteria->birthDaysDistanceInSeconds() + ) { + $personsPairMatchingCriteria = $personsPair; + } + } elseif ($finderCriteria->isByFurthestBirthDates()) { + if ($personsPair->birthDaysDistanceInSeconds() + > $personsPairMatchingCriteria->birthDaysDistanceInSeconds() + ) { + $personsPairMatchingCriteria = $personsPair; + } } } diff --git a/src/Algorithm/FinderCriteria.php b/src/Algorithm/FinderCriteria.php index 0fc7f8b..84786a3 100644 --- a/src/Algorithm/FinderCriteria.php +++ b/src/Algorithm/FinderCriteria.php @@ -4,8 +4,46 @@ namespace CodelyTV\FinderKata\Algorithm; -interface FinderCriteria +final class FinderCriteria { - const CLOSEST_BIRTHDAYS = 1; - const FURTHEST_BIRTHDAYS = 2; + const CLOSEST_BIRTH_DATES = 1; + const FURTHEST_BIRTH_DATES = 2; + + const VALID_FINDER_CRITERIA = [ + self::CLOSEST_BIRTH_DATES, + self::FURTHEST_BIRTH_DATES + ]; + + private $rawFinderCriteria; + + private function __construct(int $aRawFinderCriteria) + { + if (!in_array($aRawFinderCriteria, self::VALID_FINDER_CRITERIA, true)) { + throw new \InvalidArgumentException( + "You have specified an invalid finder criteria value: <$aRawFinderCriteria>" + ); + } + + $this->rawFinderCriteria = $aRawFinderCriteria; + } + + public static function closestBirthDates() + { + return new self(self::CLOSEST_BIRTH_DATES); + } + + public static function furthestBirthDates() + { + return new self(self::FURTHEST_BIRTH_DATES); + } + + public function isByClosestBirthDates() + { + return $this->rawFinderCriteria === self::CLOSEST_BIRTH_DATES; + } + + public function isByFurthestBirthDates() + { + return $this->rawFinderCriteria === self::FURTHEST_BIRTH_DATES; + } } diff --git a/tests/Algorithm/FinderTest.php b/tests/Algorithm/FinderTest.php index fc73248..d111e59 100644 --- a/tests/Algorithm/FinderTest.php +++ b/tests/Algorithm/FinderTest.php @@ -41,7 +41,7 @@ public function should_throw_not_enough_persons_exception_when_given_empty_list( $this->expectException(NotEnoughPersonsException::class); - $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $finder->find(FinderCriteria::closestBirthDates()); } /** @test */ @@ -52,7 +52,7 @@ public function should_throw_not_enough_persons_when_given_one_person() $this->expectException(NotEnoughPersonsException::class); - $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $finder->find(FinderCriteria::closestBirthDates()); } /** @test */ @@ -61,7 +61,7 @@ public function should_return_closest_two_for_two_people() $allPersons = [$this->sue, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::closestBirthDates()); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->greg, $personsPairFound->person2()); @@ -73,7 +73,7 @@ public function should_return_furthest_two_for_two_people() $allPersons = [$this->mike, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::furthestBirthDates()); $this->assertEquals($this->greg, $personsPairFound->person1()); $this->assertEquals($this->mike, $personsPairFound->person2()); @@ -85,7 +85,7 @@ public function should_return_furthest_two_for_four_people() $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::FURTHEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::furthestBirthDates()); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->sarah, $personsPairFound->person2()); @@ -97,7 +97,7 @@ public function should_return_closest_two_for_four_people() $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::CLOSEST_BIRTHDAYS); + $personsPairFound = $finder->find(FinderCriteria::closestBirthDates()); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->greg, $personsPairFound->person2()); From e7cd5054593aa0e7d3a09db900e1b924eb8d6a1d Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 16:34:09 +0200 Subject: [PATCH 05/10] Improve `Finder` class readability: * Extract private methods from the `Finder::find` public one * Replace `$i` and `$j` index by the more semantic ones `$currentPersonIteration` and `$personToPairIteration` * Extract `$currentPerson` and `$personToPair` explanatory variables --- src/Algorithm/Finder.php | 81 ++++++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 15 deletions(-) diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index de9e4fa..9997ef7 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -14,37 +14,88 @@ public function __construct(array $allPersons) $this->allPersons = $allPersons; } + /** + * @param FinderCriteria $finderCriteria + * + * @return PersonsPair The pair of persons matching the specified + * $finderCriteria + * + * @throws NotEnoughPersonsException + */ public function find(FinderCriteria $finderCriteria): PersonsPair { - /** @var PersonsPair[] $allPersonsPairs */ + $allPersonsPairs = $this->pairAllPersons($this->allPersons); + + $this->validateThereAreEnoughPersonsPairs($allPersonsPairs); + + $personsPairMatchingCriteria = $this->findPersonsPairMatchingCriteria( + $finderCriteria, + $allPersonsPairs + ); + + return $personsPairMatchingCriteria; + } + + /** + * @param Person[] $allPersons + * + * @return PersonsPair[] + */ + private function pairAllPersons(array $allPersons): array + { $allPersonsPairs = []; - $numberOfPersons = count($this->allPersons); + $numberOfPersons = count($allPersons); - for ($i = 0; $i < $numberOfPersons; $i++) { - for ($j = $i + 1; $j < $numberOfPersons; $j++) { + for ($currentPersonIteration = 0; + $currentPersonIteration < $numberOfPersons; + $currentPersonIteration++) { + for ($personToPairIteration = $currentPersonIteration + 1; + $personToPairIteration < $numberOfPersons; + $personToPairIteration++) { - if ($this->allPersons[$i]->birthDate() - < $this->allPersons[$j]->birthDate() - ) { - $personsPair = new PersonsPair( - $this->allPersons[$i], $this->allPersons[$j] - ); + $currentPerson = $this->allPersons[$currentPersonIteration]; + $personToPair = $this->allPersons[$personToPairIteration]; + + if ($currentPerson->birthDate() < $personToPair->birthDate()) { + $allPersonsPairs[] = + new PersonsPair($currentPerson, $personToPair); } else { - $personsPair = new PersonsPair( - $this->allPersons[$j], $this->allPersons[$i] - ); + $allPersonsPairs[] = + new PersonsPair($personToPair, $currentPerson); } - - $allPersonsPairs[] = $personsPair; } } + return $allPersonsPairs; + } + + /** + * @param PersonsPair[] $allPersonsPairs + * + * @return void + * + * @throws NotEnoughPersonsException + */ + private function validateThereAreEnoughPersonsPairs(array $allPersonsPairs) + { $thereAreNoPersonsPairs = count($allPersonsPairs) < 1; if ($thereAreNoPersonsPairs) { throw new NotEnoughPersonsException(); } + } + /** + * @param FinderCriteria $finderCriteria + * @param PersonsPair[] $allPersonsPairs + * + * @return PersonsPair + */ + private function findPersonsPairMatchingCriteria( + FinderCriteria $finderCriteria, + array $allPersonsPairs + ): PersonsPair + { $personsPairMatchingCriteria = $allPersonsPairs[0]; foreach ($allPersonsPairs as $personsPair) { From 8fba6ef71588c750041d551e41019005860dd20e Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 16:56:47 +0200 Subject: [PATCH 06/10] Make the Finder OCP (Open/Closed SOLID Principle) compliant * Refactor the `FinderCriteria` Value Object into an `interface` and implement it from two different models `ClosestBirthDateCriteria` and `FurthestBirthDateCriteria`. This classes implement the logic to determine which `PersonsPair` `hasMorePriority`. So if we want to add another different criteria, we just need to implement the `PersonsPairCriteria` interface and pass it to the `Finder:: find` method without modifying any `switch` nor `if`/`elseif`. --- src/Algorithm/Finder.php | 38 +++++++------- src/Algorithm/FinderCriteria.php | 49 ------------------- .../PersonsPair/ClosestBirthDateCriteria.php | 18 +++++++ .../PersonsPair/FurthestBirthDateCriteria.php | 18 +++++++ .../Model/PersonsPair/PersonsPairCriteria.php | 23 +++++++++ tests/Algorithm/FinderTest.php | 33 ++++++++++--- 6 files changed, 102 insertions(+), 77 deletions(-) delete mode 100644 src/Algorithm/FinderCriteria.php create mode 100644 src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php create mode 100644 src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php create mode 100644 src/Domain/Model/PersonsPair/PersonsPairCriteria.php diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index 9997ef7..917d0f5 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -4,6 +4,8 @@ namespace CodelyTV\FinderKata\Algorithm; +use CodelyTV\FinderKata\Domain\Model\PersonsPair\PersonsPairCriteria; + final class Finder { /** @var Person[] */ @@ -15,14 +17,14 @@ public function __construct(array $allPersons) } /** - * @param FinderCriteria $finderCriteria + * @param PersonsPairCriteria $finderCriteria * * @return PersonsPair The pair of persons matching the specified * $finderCriteria * * @throws NotEnoughPersonsException */ - public function find(FinderCriteria $finderCriteria): PersonsPair + public function find(PersonsPairCriteria $finderCriteria): PersonsPair { $allPersonsPairs = $this->pairAllPersons($this->allPersons); @@ -86,34 +88,28 @@ private function validateThereAreEnoughPersonsPairs(array $allPersonsPairs) } /** - * @param FinderCriteria $finderCriteria - * @param PersonsPair[] $allPersonsPairs + * @param PersonsPairCriteria $personsPairPriorityCriteria + * @param PersonsPair[] $allPersonsPairs * * @return PersonsPair */ private function findPersonsPairMatchingCriteria( - FinderCriteria $finderCriteria, + PersonsPairCriteria $personsPairPriorityCriteria, array $allPersonsPairs ): PersonsPair { - $personsPairMatchingCriteria = $allPersonsPairs[0]; - - foreach ($allPersonsPairs as $personsPair) { - if ($finderCriteria->isByClosestBirthDates()) { - if ($personsPair->birthDaysDistanceInSeconds() - < $personsPairMatchingCriteria->birthDaysDistanceInSeconds() - ) { - $personsPairMatchingCriteria = $personsPair; - } - } elseif ($finderCriteria->isByFurthestBirthDates()) { - if ($personsPair->birthDaysDistanceInSeconds() - > $personsPairMatchingCriteria->birthDaysDistanceInSeconds() - ) { - $personsPairMatchingCriteria = $personsPair; - } + $bestPersonsPair = $allPersonsPairs[0]; + + foreach ($allPersonsPairs as $personsPairCandidate) { + if ($personsPairPriorityCriteria->hasMorePriority( + $bestPersonsPair, + $personsPairCandidate + ) + ) { + $bestPersonsPair = $personsPairCandidate; } } - return $personsPairMatchingCriteria; + return $bestPersonsPair; } } diff --git a/src/Algorithm/FinderCriteria.php b/src/Algorithm/FinderCriteria.php deleted file mode 100644 index 84786a3..0000000 --- a/src/Algorithm/FinderCriteria.php +++ /dev/null @@ -1,49 +0,0 @@ -" - ); - } - - $this->rawFinderCriteria = $aRawFinderCriteria; - } - - public static function closestBirthDates() - { - return new self(self::CLOSEST_BIRTH_DATES); - } - - public static function furthestBirthDates() - { - return new self(self::FURTHEST_BIRTH_DATES); - } - - public function isByClosestBirthDates() - { - return $this->rawFinderCriteria === self::CLOSEST_BIRTH_DATES; - } - - public function isByFurthestBirthDates() - { - return $this->rawFinderCriteria === self::FURTHEST_BIRTH_DATES; - } -} diff --git a/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php b/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php new file mode 100644 index 0000000..c104b17 --- /dev/null +++ b/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php @@ -0,0 +1,18 @@ +birthDaysDistanceInSeconds() + < $basePersonsPair->birthDaysDistanceInSeconds(); + } +} diff --git a/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php b/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php new file mode 100644 index 0000000..ac955cf --- /dev/null +++ b/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php @@ -0,0 +1,18 @@ +birthDaysDistanceInSeconds() + > $basePersonsPair->birthDaysDistanceInSeconds(); + } +} diff --git a/src/Domain/Model/PersonsPair/PersonsPairCriteria.php b/src/Domain/Model/PersonsPair/PersonsPairCriteria.php new file mode 100644 index 0000000..1b20d4a --- /dev/null +++ b/src/Domain/Model/PersonsPair/PersonsPairCriteria.php @@ -0,0 +1,23 @@ +expectException(NotEnoughPersonsException::class); - $finder->find(FinderCriteria::closestBirthDates()); + $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); + + $finder->find($closestBirthDatePersonsPairsCriteria); } /** @test */ @@ -52,7 +55,9 @@ public function should_throw_not_enough_persons_when_given_one_person() $this->expectException(NotEnoughPersonsException::class); - $finder->find(FinderCriteria::closestBirthDates()); + $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); + + $finder->find($closestBirthDatePersonsPairsCriteria); } /** @test */ @@ -61,7 +66,10 @@ public function should_return_closest_two_for_two_people() $allPersons = [$this->sue, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::closestBirthDates()); + $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); + + $personsPairFound = + $finder->find($closestBirthDatePersonsPairsCriteria); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->greg, $personsPairFound->person2()); @@ -73,7 +81,11 @@ public function should_return_furthest_two_for_two_people() $allPersons = [$this->mike, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::furthestBirthDates()); + $furthestBirthDatePersonsPairsCriteria = + new FurthestBirthDateCriteria(); + + $personsPairFound = + $finder->find($furthestBirthDatePersonsPairsCriteria); $this->assertEquals($this->greg, $personsPairFound->person1()); $this->assertEquals($this->mike, $personsPairFound->person2()); @@ -85,7 +97,11 @@ public function should_return_furthest_two_for_four_people() $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::furthestBirthDates()); + $furthestBirthDatePersonsPairsCriteria = + new FurthestBirthDateCriteria(); + + $personsPairFound = + $finder->find($furthestBirthDatePersonsPairsCriteria); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->sarah, $personsPairFound->person2()); @@ -97,7 +113,10 @@ public function should_return_closest_two_for_four_people() $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; $finder = new Finder($allPersons); - $personsPairFound = $finder->find(FinderCriteria::closestBirthDates()); + $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); + + $personsPairFound = + $finder->find($closestBirthDatePersonsPairsCriteria); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->greg, $personsPairFound->person2()); From 597457661c5bf6d0336d05b417c268026afd83bd Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 17:03:47 +0200 Subject: [PATCH 07/10] Move the `$allPersons` input array from the `Finder` constructor to the `find` method. This way we don't need to create 2 different `Finder` instances in order to execute the algorithm with 2 different inputs, and more importantly, we leave the constructor ready in order to only receive collaborator as parameters --- src/Algorithm/Finder.php | 22 ++++++++-------------- tests/Algorithm/FinderTest.php | 24 ++++++++++++------------ 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index 917d0f5..f0cda6d 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -8,25 +8,19 @@ final class Finder { - /** @var Person[] */ - private $allPersons; - - public function __construct(array $allPersons) - { - $this->allPersons = $allPersons; - } - /** + * @param Person[] $allPersons * @param PersonsPairCriteria $finderCriteria * * @return PersonsPair The pair of persons matching the specified * $finderCriteria - * - * @throws NotEnoughPersonsException */ - public function find(PersonsPairCriteria $finderCriteria): PersonsPair + public function find( + array $allPersons, + PersonsPairCriteria $finderCriteria + ): PersonsPair { - $allPersonsPairs = $this->pairAllPersons($this->allPersons); + $allPersonsPairs = $this->pairAllPersons($allPersons); $this->validateThereAreEnoughPersonsPairs($allPersonsPairs); @@ -56,8 +50,8 @@ private function pairAllPersons(array $allPersons): array $personToPairIteration < $numberOfPersons; $personToPairIteration++) { - $currentPerson = $this->allPersons[$currentPersonIteration]; - $personToPair = $this->allPersons[$personToPairIteration]; + $currentPerson = $allPersons[$currentPersonIteration]; + $personToPair = $allPersons[$personToPairIteration]; if ($currentPerson->birthDate() < $personToPair->birthDate()) { $allPersonsPairs[] = diff --git a/tests/Algorithm/FinderTest.php b/tests/Algorithm/FinderTest.php index 61809b2..b88aace 100644 --- a/tests/Algorithm/FinderTest.php +++ b/tests/Algorithm/FinderTest.php @@ -38,38 +38,38 @@ public function should_throw_not_enough_persons_exception_when_given_empty_list( ) { $allPersons = []; - $finder = new Finder($allPersons); + $finder = new Finder(); $this->expectException(NotEnoughPersonsException::class); $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); - $finder->find($closestBirthDatePersonsPairsCriteria); + $finder->find($allPersons, $closestBirthDatePersonsPairsCriteria); } /** @test */ public function should_throw_not_enough_persons_when_given_one_person() { $allPersons = [$this->sue]; - $finder = new Finder($allPersons); + $finder = new Finder(); $this->expectException(NotEnoughPersonsException::class); $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); - $finder->find($closestBirthDatePersonsPairsCriteria); + $finder->find($allPersons, $closestBirthDatePersonsPairsCriteria); } /** @test */ public function should_return_closest_two_for_two_people() { $allPersons = [$this->sue, $this->greg]; - $finder = new Finder($allPersons); + $finder = new Finder(); $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); $personsPairFound = - $finder->find($closestBirthDatePersonsPairsCriteria); + $finder->find($allPersons, $closestBirthDatePersonsPairsCriteria); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->greg, $personsPairFound->person2()); @@ -79,13 +79,13 @@ public function should_return_closest_two_for_two_people() public function should_return_furthest_two_for_two_people() { $allPersons = [$this->mike, $this->greg]; - $finder = new Finder($allPersons); + $finder = new Finder(); $furthestBirthDatePersonsPairsCriteria = new FurthestBirthDateCriteria(); $personsPairFound = - $finder->find($furthestBirthDatePersonsPairsCriteria); + $finder->find($allPersons, $furthestBirthDatePersonsPairsCriteria); $this->assertEquals($this->greg, $personsPairFound->person1()); $this->assertEquals($this->mike, $personsPairFound->person2()); @@ -95,13 +95,13 @@ public function should_return_furthest_two_for_two_people() public function should_return_furthest_two_for_four_people() { $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; - $finder = new Finder($allPersons); + $finder = new Finder(); $furthestBirthDatePersonsPairsCriteria = new FurthestBirthDateCriteria(); $personsPairFound = - $finder->find($furthestBirthDatePersonsPairsCriteria); + $finder->find($allPersons, $furthestBirthDatePersonsPairsCriteria); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->sarah, $personsPairFound->person2()); @@ -111,12 +111,12 @@ public function should_return_furthest_two_for_four_people() public function should_return_closest_two_for_four_people() { $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; - $finder = new Finder($allPersons); + $finder = new Finder(); $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); $personsPairFound = - $finder->find($closestBirthDatePersonsPairsCriteria); + $finder->find($allPersons, $closestBirthDatePersonsPairsCriteria); $this->assertEquals($this->sue, $personsPairFound->person1()); $this->assertEquals($this->greg, $personsPairFound->person2()); From 4b014cbbe3253af0fcfde1d7c07a1031fcbca840 Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 17:19:50 +0200 Subject: [PATCH 08/10] Extract the responsibility of pair all the `Person` received as the `Finder::find` input into the new `PersonsPairer` interface allowing also OCP applications on that end --- src/Algorithm/Finder.php | 45 +++++-------------- .../PersonsPair/ClosestBirthDateCriteria.php | 1 - .../PersonsPair/FurthestBirthDateCriteria.php | 1 - .../Service/PersonsPair/PersonsPairer.php | 18 ++++++++ .../PersonsPair/SequentialPersonsPairer.php | 37 +++++++++++++++ tests/Algorithm/FinderTest.php | 27 ++++++++--- 6 files changed, 85 insertions(+), 44 deletions(-) create mode 100644 src/Domain/Service/PersonsPair/PersonsPairer.php create mode 100644 src/Domain/Service/PersonsPair/SequentialPersonsPairer.php diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index f0cda6d..1bc71c5 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -5,9 +5,18 @@ namespace CodelyTV\FinderKata\Algorithm; use CodelyTV\FinderKata\Domain\Model\PersonsPair\PersonsPairCriteria; +use CodelyTV\FinderKata\Domain\Service\PersonsPair\PersonsPairer; final class Finder { + /** @var PersonsPairer */ + private $personsPairer; + + public function __construct(PersonsPairer $aPersonsPairer) + { + $this->personsPairer = $aPersonsPairer; + } + /** * @param Person[] $allPersons * @param PersonsPairCriteria $finderCriteria @@ -20,7 +29,7 @@ public function find( PersonsPairCriteria $finderCriteria ): PersonsPair { - $allPersonsPairs = $this->pairAllPersons($allPersons); + $allPersonsPairs = $this->personsPairer->pair($allPersons); $this->validateThereAreEnoughPersonsPairs($allPersonsPairs); @@ -32,40 +41,6 @@ public function find( return $personsPairMatchingCriteria; } - /** - * @param Person[] $allPersons - * - * @return PersonsPair[] - */ - private function pairAllPersons(array $allPersons): array - { - $allPersonsPairs = []; - - $numberOfPersons = count($allPersons); - - for ($currentPersonIteration = 0; - $currentPersonIteration < $numberOfPersons; - $currentPersonIteration++) { - for ($personToPairIteration = $currentPersonIteration + 1; - $personToPairIteration < $numberOfPersons; - $personToPairIteration++) { - - $currentPerson = $allPersons[$currentPersonIteration]; - $personToPair = $allPersons[$personToPairIteration]; - - if ($currentPerson->birthDate() < $personToPair->birthDate()) { - $allPersonsPairs[] = - new PersonsPair($currentPerson, $personToPair); - } else { - $allPersonsPairs[] = - new PersonsPair($personToPair, $currentPerson); - } - } - } - - return $allPersonsPairs; - } - /** * @param PersonsPair[] $allPersonsPairs * diff --git a/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php b/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php index c104b17..4ecee0c 100644 --- a/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php +++ b/src/Domain/Model/PersonsPair/ClosestBirthDateCriteria.php @@ -6,7 +6,6 @@ final class ClosestBirthDateCriteria implements PersonsPairCriteria { - /** @{InheritDoc} */ public function hasMorePriority( PersonsPair $basePersonsPair, PersonsPair $candidatePersonsPair diff --git a/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php b/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php index ac955cf..bc14017 100644 --- a/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php +++ b/src/Domain/Model/PersonsPair/FurthestBirthDateCriteria.php @@ -6,7 +6,6 @@ final class FurthestBirthDateCriteria implements PersonsPairCriteria { - /** @{InheritDoc} */ public function hasMorePriority( PersonsPair $basePersonsPair, PersonsPair $candidatePersonsPair diff --git a/src/Domain/Service/PersonsPair/PersonsPairer.php b/src/Domain/Service/PersonsPair/PersonsPairer.php new file mode 100644 index 0000000..052012e --- /dev/null +++ b/src/Domain/Service/PersonsPair/PersonsPairer.php @@ -0,0 +1,18 @@ +birthDate() < $personToPair->birthDate()) { + $allPersonsPairs[] = + new PersonsPair($currentPerson, $personToPair); + } else { + $allPersonsPairs[] = + new PersonsPair($personToPair, $currentPerson); + } + } + } + + return $allPersonsPairs; + } +} diff --git a/tests/Algorithm/FinderTest.php b/tests/Algorithm/FinderTest.php index b88aace..32dfde8 100644 --- a/tests/Algorithm/FinderTest.php +++ b/tests/Algorithm/FinderTest.php @@ -9,6 +9,7 @@ use CodelyTV\FinderKata\Algorithm\Person; use CodelyTV\FinderKata\Domain\Model\PersonsPair\ClosestBirthDateCriteria; use CodelyTV\FinderKata\Domain\Model\PersonsPair\FurthestBirthDateCriteria; +use CodelyTV\FinderKata\Domain\Service\PersonsPair\SequentialPersonsPairer; use PHPUnit\Framework\TestCase; final class FinderTest extends TestCase @@ -37,8 +38,10 @@ protected function setUp() public function should_throw_not_enough_persons_exception_when_given_empty_list( ) { + $sequentialPersonsPairer = new SequentialPersonsPairer(); + $finder = new Finder($sequentialPersonsPairer); + $allPersons = []; - $finder = new Finder(); $this->expectException(NotEnoughPersonsException::class); @@ -50,8 +53,10 @@ public function should_throw_not_enough_persons_exception_when_given_empty_list( /** @test */ public function should_throw_not_enough_persons_when_given_one_person() { + $sequentialPersonsPairer = new SequentialPersonsPairer(); + $finder = new Finder($sequentialPersonsPairer); + $allPersons = [$this->sue]; - $finder = new Finder(); $this->expectException(NotEnoughPersonsException::class); @@ -63,8 +68,10 @@ public function should_throw_not_enough_persons_when_given_one_person() /** @test */ public function should_return_closest_two_for_two_people() { + $sequentialPersonsPairer = new SequentialPersonsPairer(); + $finder = new Finder($sequentialPersonsPairer); + $allPersons = [$this->sue, $this->greg]; - $finder = new Finder(); $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); @@ -78,8 +85,10 @@ public function should_return_closest_two_for_two_people() /** @test */ public function should_return_furthest_two_for_two_people() { + $sequentialPersonsPairer = new SequentialPersonsPairer(); + $finder = new Finder($sequentialPersonsPairer); + $allPersons = [$this->mike, $this->greg]; - $finder = new Finder(); $furthestBirthDatePersonsPairsCriteria = new FurthestBirthDateCriteria(); @@ -94,8 +103,10 @@ public function should_return_furthest_two_for_two_people() /** @test */ public function should_return_furthest_two_for_four_people() { + $sequentialPersonsPairer = new SequentialPersonsPairer(); + $finder = new Finder($sequentialPersonsPairer); + $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; - $finder = new Finder(); $furthestBirthDatePersonsPairsCriteria = new FurthestBirthDateCriteria(); @@ -110,9 +121,11 @@ public function should_return_furthest_two_for_four_people() /** @test */ public function should_return_closest_two_for_four_people() { - $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; - $finder = new Finder(); + $sequentialPersonsPairer = new SequentialPersonsPairer(); + $finder = new Finder($sequentialPersonsPairer); + $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; + $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); $personsPairFound = From 097b1b93af5d406221e54f8a1b40cfea13000c3a Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 17:35:06 +0200 Subject: [PATCH 09/10] Extract the responsibility of constructing the `PersonsPair` from the `PersonsPairer` to its own `PersonsPairFactory`. This way we can mix different `PersonsPairer` implementations without repeating the `PersonsPair` construction logic --- .../PersonsPair/PersonsPairFactory.php | 11 +++++++ .../PersonsPair/SequentialPersonsPairer.php | 24 ++++++++------- .../YoungerFirstPersonsPairFactory.php | 18 ++++++++++++ tests/Algorithm/FinderTest.php | 29 ++++++++++++++----- 4 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 src/Domain/Service/PersonsPair/PersonsPairFactory.php create mode 100644 src/Domain/Service/PersonsPair/YoungerFirstPersonsPairFactory.php diff --git a/src/Domain/Service/PersonsPair/PersonsPairFactory.php b/src/Domain/Service/PersonsPair/PersonsPairFactory.php new file mode 100644 index 0000000..407e49f --- /dev/null +++ b/src/Domain/Service/PersonsPair/PersonsPairFactory.php @@ -0,0 +1,11 @@ +personsPairFactory = $aPersonsPairFactory; + } + + public function pair(array $allPersons): array + { $allPersonsPairs = []; $numberOfPersons = count($allPersons); @@ -22,13 +29,10 @@ public function pair(array $allPersons): array { $currentPerson = $allPersons[$currentPersonIteration]; $personToPair = $allPersons[$personToPairIteration]; - if ($currentPerson->birthDate() < $personToPair->birthDate()) { - $allPersonsPairs[] = - new PersonsPair($currentPerson, $personToPair); - } else { - $allPersonsPairs[] = - new PersonsPair($personToPair, $currentPerson); - } + $allPersonsPairs[] = $this->personsPairFactory->__invoke( + $currentPerson, + $personToPair + ); } } diff --git a/src/Domain/Service/PersonsPair/YoungerFirstPersonsPairFactory.php b/src/Domain/Service/PersonsPair/YoungerFirstPersonsPairFactory.php new file mode 100644 index 0000000..fe3e44f --- /dev/null +++ b/src/Domain/Service/PersonsPair/YoungerFirstPersonsPairFactory.php @@ -0,0 +1,18 @@ +birthDate() < $person2->birthDate()) { + return new PersonsPair($person1, $person2); + } else { + return new PersonsPair($person2, $person1); + } + } +} diff --git a/tests/Algorithm/FinderTest.php b/tests/Algorithm/FinderTest.php index 32dfde8..1816f58 100644 --- a/tests/Algorithm/FinderTest.php +++ b/tests/Algorithm/FinderTest.php @@ -10,6 +10,7 @@ use CodelyTV\FinderKata\Domain\Model\PersonsPair\ClosestBirthDateCriteria; use CodelyTV\FinderKata\Domain\Model\PersonsPair\FurthestBirthDateCriteria; use CodelyTV\FinderKata\Domain\Service\PersonsPair\SequentialPersonsPairer; +use CodelyTV\FinderKata\Domain\Service\PersonsPair\YoungerFirstPersonsPairFactory; use PHPUnit\Framework\TestCase; final class FinderTest extends TestCase @@ -38,8 +39,10 @@ protected function setUp() public function should_throw_not_enough_persons_exception_when_given_empty_list( ) { - $sequentialPersonsPairer = new SequentialPersonsPairer(); - $finder = new Finder($sequentialPersonsPairer); + $youngerFirstPersonsPairFactory = new YoungerFirstPersonsPairFactory(); + $sequentialPersonsPairer = + new SequentialPersonsPairer($youngerFirstPersonsPairFactory); + $finder = new Finder($sequentialPersonsPairer); $allPersons = []; @@ -53,7 +56,9 @@ public function should_throw_not_enough_persons_exception_when_given_empty_list( /** @test */ public function should_throw_not_enough_persons_when_given_one_person() { - $sequentialPersonsPairer = new SequentialPersonsPairer(); + $youngerFirstPersonsPairFactory = new YoungerFirstPersonsPairFactory(); + $sequentialPersonsPairer = + new SequentialPersonsPairer($youngerFirstPersonsPairFactory); $finder = new Finder($sequentialPersonsPairer); $allPersons = [$this->sue]; @@ -68,7 +73,9 @@ public function should_throw_not_enough_persons_when_given_one_person() /** @test */ public function should_return_closest_two_for_two_people() { - $sequentialPersonsPairer = new SequentialPersonsPairer(); + $youngerFirstPersonsPairFactory = new YoungerFirstPersonsPairFactory(); + $sequentialPersonsPairer = + new SequentialPersonsPairer($youngerFirstPersonsPairFactory); $finder = new Finder($sequentialPersonsPairer); $allPersons = [$this->sue, $this->greg]; @@ -85,7 +92,9 @@ public function should_return_closest_two_for_two_people() /** @test */ public function should_return_furthest_two_for_two_people() { - $sequentialPersonsPairer = new SequentialPersonsPairer(); + $youngerFirstPersonsPairFactory = new YoungerFirstPersonsPairFactory(); + $sequentialPersonsPairer = + new SequentialPersonsPairer($youngerFirstPersonsPairFactory); $finder = new Finder($sequentialPersonsPairer); $allPersons = [$this->mike, $this->greg]; @@ -103,7 +112,9 @@ public function should_return_furthest_two_for_two_people() /** @test */ public function should_return_furthest_two_for_four_people() { - $sequentialPersonsPairer = new SequentialPersonsPairer(); + $youngerFirstPersonsPairFactory = new YoungerFirstPersonsPairFactory(); + $sequentialPersonsPairer = + new SequentialPersonsPairer($youngerFirstPersonsPairFactory); $finder = new Finder($sequentialPersonsPairer); $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; @@ -121,11 +132,13 @@ public function should_return_furthest_two_for_four_people() /** @test */ public function should_return_closest_two_for_four_people() { - $sequentialPersonsPairer = new SequentialPersonsPairer(); + $youngerFirstPersonsPairFactory = new YoungerFirstPersonsPairFactory(); + $sequentialPersonsPairer = + new SequentialPersonsPairer($youngerFirstPersonsPairFactory); $finder = new Finder($sequentialPersonsPairer); $allPersons = [$this->sue, $this->sarah, $this->mike, $this->greg]; - + $closestBirthDatePersonsPairsCriteria = new ClosestBirthDateCriteria(); $personsPairFound = From 178f30d31a6491149e9f619cf336900bacba2631 Mon Sep 17 00:00:00 2001 From: Javier Ferrer Date: Mon, 15 Aug 2016 17:37:17 +0200 Subject: [PATCH 10/10] Rename `PersonsPairer::pair` method to `PersonsPairer::__invoke` for consistency. We don't need the method name semantics. We're doing little pieces so the semantics could be in the interface or class name by its own --- src/Algorithm/Finder.php | 2 +- src/Domain/Service/PersonsPair/PersonsPairer.php | 2 +- src/Domain/Service/PersonsPair/SequentialPersonsPairer.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Algorithm/Finder.php b/src/Algorithm/Finder.php index 1bc71c5..ac79175 100644 --- a/src/Algorithm/Finder.php +++ b/src/Algorithm/Finder.php @@ -29,7 +29,7 @@ public function find( PersonsPairCriteria $finderCriteria ): PersonsPair { - $allPersonsPairs = $this->personsPairer->pair($allPersons); + $allPersonsPairs = $this->personsPairer->__invoke($allPersons); $this->validateThereAreEnoughPersonsPairs($allPersonsPairs); diff --git a/src/Domain/Service/PersonsPair/PersonsPairer.php b/src/Domain/Service/PersonsPair/PersonsPairer.php index 052012e..5318a7a 100644 --- a/src/Domain/Service/PersonsPair/PersonsPairer.php +++ b/src/Domain/Service/PersonsPair/PersonsPairer.php @@ -14,5 +14,5 @@ interface PersonsPairer * * @return PersonsPair[] */ - public function pair(array $allPersons): array; + public function __invoke(array $allPersons): array; } diff --git a/src/Domain/Service/PersonsPair/SequentialPersonsPairer.php b/src/Domain/Service/PersonsPair/SequentialPersonsPairer.php index d5deef5..c55249c 100644 --- a/src/Domain/Service/PersonsPair/SequentialPersonsPairer.php +++ b/src/Domain/Service/PersonsPair/SequentialPersonsPairer.php @@ -12,7 +12,7 @@ public function __construct(PersonsPairFactory $aPersonsPairFactory) $this->personsPairFactory = $aPersonsPairFactory; } - public function pair(array $allPersons): array + public function __invoke(array $allPersons): array { $allPersonsPairs = [];