From b7f6484c70d3297c04ccb2ffeae5e83da5b79286 Mon Sep 17 00:00:00 2001 From: Andre Wyrwa Date: Fri, 26 May 2017 16:19:12 +1000 Subject: [PATCH] improved prefix handling in LastnameMapper; refactors to improve static analysis ratings --- src/Mapper/FirstnameMapper.php | 54 ++++++++++++++---------- src/Mapper/InitialMapper.php | 17 +++++++- src/Mapper/LastnameMapper.php | 65 +++++++++++++++++++++++++---- src/Mapper/MiddlenameMapper.php | 1 - src/Mapper/SuffixMapper.php | 40 ++++++++++++++---- src/Parser.php | 40 ++++++++++++------ tests/Mapper/LastnameMapperTest.php | 20 +++++++++ tests/ParserTest.php | 8 ++++ 8 files changed, 193 insertions(+), 52 deletions(-) diff --git a/src/Mapper/FirstnameMapper.php b/src/Mapper/FirstnameMapper.php index dd6da22..e147687 100644 --- a/src/Mapper/FirstnameMapper.php +++ b/src/Mapper/FirstnameMapper.php @@ -22,11 +22,39 @@ public function map(array $parts) return [$this->handleSinglePart($parts[0])]; } - $length = count($parts); - $start = $this->getStartIndex($parts); + $pos = $this->findFirstnamePosition($parts); + + if (null !== $pos) { + $parts[$pos] = new Firstname($parts[$pos]); + } + + return $parts; + } + + /** + * @param $part + * @return Firstname + */ + protected function handleSinglePart($part) + { + if ($part instanceof AbstractPart) { + return $part; + } + + return new Firstname($part); + } + /** + * @param array $parts + * @return int|null + */ + protected function findFirstnamePosition(array $parts) + { $pos = null; + $length = count($parts); + $start = $this->getStartIndex($parts); + for ($k = $start; $k < $length; $k++) { $part = $parts[$k]; @@ -42,28 +70,10 @@ public function map(array $parts) continue; } - $pos = $k; - break; - } - - if (null !== $pos) { - $parts[$pos] = new Firstname($parts[$pos]); + return $k; } - return $parts; - } - - /** - * @param $part - * @return Firstname - */ - protected function handleSinglePart($part) - { - if ($part instanceof AbstractPart) { - return $part; - } - - return new Firstname($part); + return $pos; } /** diff --git a/src/Mapper/InitialMapper.php b/src/Mapper/InitialMapper.php index 724bc89..dd4bfad 100644 --- a/src/Mapper/InitialMapper.php +++ b/src/Mapper/InitialMapper.php @@ -23,11 +23,26 @@ public function map(array $parts) continue; } - if ((strlen($part) == 1) || (strlen($part) == 2 && substr($part, -1) === '.')) { + if ($this->isInitial($part)) { $parts[$k] = new Initial($part); } } return $parts; } + + /** + * @param string $part + * @return bool + */ + protected function isInitial(string $part): bool + { + $length = strlen($part); + + if (1 === $length) { + return true; + } + + return ($length === 2 && substr($part, -1) === '.'); + } } diff --git a/src/Mapper/LastnameMapper.php b/src/Mapper/LastnameMapper.php index 1352aed..af27107 100644 --- a/src/Mapper/LastnameMapper.php +++ b/src/Mapper/LastnameMapper.php @@ -33,6 +33,17 @@ public function map(array $parts) $parts = array_reverse($parts); + $parts = $this->mapReversedParts($parts); + + return array_reverse($parts); + } + + /** + * @param array $parts + * @return array + */ + protected function mapReversedParts(array $parts): array + { foreach ($parts as $k => $part) { if ($part instanceof Suffix) { continue; @@ -42,19 +53,55 @@ public function map(array $parts) break; } - if (Lastname::isPrefix($part)) { - if (isset($parts[$k-1]) && $parts[$k-1] instanceof Lastname) { - if ($this->hasUnmappedPartsBefore(array_reverse($parts), count($parts) - $k - 1)) { - $parts[$k] = new Lastname($part); - } + $originalIndex = count($parts) - $k - 1; + $originalParts = array_reverse($parts); + + if ($this->isFollowedByLastnamePart($originalParts, $originalIndex)) { + if ($this->isApplicablePrefix($originalParts, $originalIndex)) { + $parts[$k] = new Lastname($part); + continue; } - } elseif (!isset($parts[$k-1]) || !($parts[$k-1] instanceof Lastname)) { - $parts[$k] = new Lastname($part); - } else { break; } + + $parts[$k] = new Lastname($part); } - return array_reverse($parts); + return $parts; + } + + /** + * @param array $parts + * @param int $index + * @return bool + */ + protected function isFollowedByLastnamePart(array $parts, int $index): bool + { + $next = $index + 1; + + return (isset($parts[$next]) && $parts[$next] instanceof Lastname); + } + + /** + * Assuming that the part at the given index is matched as a prefix, + * determines if the prefix should be applied to the lastname. + * + * We only apply it to the lastname if we already have at least one + * lastname part and there are other parts left in + * the name (this effectively prioritises firstname over prefix matching). + * + * This expects the parts array and index to be in the original order. + * + * @param array $parts + * @param int $index + * @return bool + */ + protected function isApplicablePrefix(array $parts, int $index): bool + { + if (!Lastname::isPrefix($parts[$index])) { + return false; + } + + return $this->hasUnmappedPartsBefore($parts, $index); } } diff --git a/src/Mapper/MiddlenameMapper.php b/src/Mapper/MiddlenameMapper.php index 7163787..dc11ae0 100644 --- a/src/Mapper/MiddlenameMapper.php +++ b/src/Mapper/MiddlenameMapper.php @@ -21,7 +21,6 @@ public function map(array $parts) return $parts; } - // skip to after salutation $length = count($parts); $start = $this->findFirstMapped(Firstname::class, $parts); diff --git a/src/Mapper/SuffixMapper.php b/src/Mapper/SuffixMapper.php index 8878af7..d1ad559 100644 --- a/src/Mapper/SuffixMapper.php +++ b/src/Mapper/SuffixMapper.php @@ -22,7 +22,7 @@ class SuffixMapper extends AbstractMapper */ public function map(array $parts) { - if ($this->options['match_single'] && count($parts) == 1 && Suffix::isSuffix($parts[0])) { + if ($this->isMatchingSinglePart($parts)) { $parts[0] = new Suffix($parts[0]); return $parts; } @@ -32,17 +32,43 @@ public function map(array $parts) for ($k = $start; $k > 1; $k--) { $part = $parts[$k]; - if ($part instanceof AbstractPart) { + if (!$this->isSuffix($part)) { break; } - if (Suffix::isSuffix($part)) { - $parts[$k] = new Suffix($part); - } else { - break; - } + $parts[$k] = new Suffix($part); } return $parts; } + + /** + * @param $parts + * @return bool + */ + protected function isMatchingSinglePart($parts) + { + if (!$this->options['match_single']) { + return false; + } + + if (1 !== count($parts)) { + return false; + } + + return $this->isSuffix($parts[0]); + } + + /** + * @param $part + * @return bool + */ + protected function isSuffix($part): bool + { + if ($part instanceof AbstractPart) { + return false; + } + + return (Suffix::isSuffix($part)); + } } diff --git a/src/Parser.php b/src/Parser.php index a6aa86f..c8b3b7a 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -30,10 +30,10 @@ class Parser * - surname / last name * - suffix (II, Phd, Jr, etc) * - * @param $full_name + * @param string $name * @return Name */ - public function parse($name) + public function parse($name): Name { $name = $this->normalize($name); @@ -60,8 +60,21 @@ public function parse($name) */ protected function parseSplitName($left, $right) { - $first = new Parser(); - $first->setMappers([ + $parts = array_merge( + $this->getLeftSplitNameParser()->parse($left)->getParts(), + $this->getRightSplitNameParser()->parse($right)->getParts() + ); + + return new Name($parts); + } + + /** + * @return Parser + */ + protected function getLeftSplitNameParser() + { + $parser = new Parser(); + $parser->setMappers([ new SalutationMapper(), new SuffixMapper(), new LastnameMapper(['match_single' => true]), @@ -69,8 +82,16 @@ protected function parseSplitName($left, $right) new MiddlenameMapper(), ]); - $second = new Parser(); - $second->setMappers([ + return $parser; + } + + /** + * @return Parser + */ + protected function getRightSplitNameParser() + { + $parser = new Parser(); + $parser->setMappers([ new SalutationMapper(), new SuffixMapper(['match_single' => true]), new NicknameMapper(), @@ -79,12 +100,7 @@ protected function parseSplitName($left, $right) new MiddlenameMapper(), ]); - $parts = array_merge( - $first->parse($left)->getParts(), - $second->parse($right)->getParts() - ); - - return new Name($parts); + return $parser; } /** diff --git a/tests/Mapper/LastnameMapperTest.php b/tests/Mapper/LastnameMapperTest.php index 072545b..182b901 100644 --- a/tests/Mapper/LastnameMapperTest.php +++ b/tests/Mapper/LastnameMapperTest.php @@ -74,6 +74,26 @@ public function provider() new Lastname('Huong'), ], ], + [ + 'input' => [ + new Salutation('Mr'), + 'Von', + ], + 'expectation' => [ + new Salutation('Mr'), + new Lastname('Von'), + ], + ], + [ + 'input' => [ + 'Mr', + 'Von', + ], + 'expectation' => [ + 'Mr', + new Lastname('Von'), + ], + ], ]; } } diff --git a/tests/ParserTest.php b/tests/ParserTest.php index aa676ef..238bc27 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -351,6 +351,14 @@ public function provider() 'suffix' => 'MD' ] ], + [ + 'Kirk, James T.', + [ + 'firstname' => 'James', + 'initials' => 'T.', + 'lastname' => 'Kirk', + ], + ], ]; }