RelationshipController::calculateRelationships() - Code Metrics - Inspection of "Typo" - fisharebest/webtrees - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 1.7 ( f5d842...5315e2 )

by Greg

created 2016-12-24 08:33 UTC

RelationshipController::calculateRelationships() C

↳ Parent: RelationshipController

Complexity

Conditions	13
Paths	72

Size

Total Lines	69
Code Lines	43

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	13
eloc	43
nc	72
nop	4
dl	0
loc	69
rs	5.6955
c	0
b	0
f	0

How to fix Long Method Complexity

<?php
/**
 * webtrees: online genealogy
 * Copyright (C) 2016 webtrees development team
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * (at your option) any later version.
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU General Public License for more details.
 * You should have received a copy of the GNU General Public License
 * along with this program. If not, see <http://www.gnu.org/licenses/>.
 */
namespace Fisharebest\Webtrees\Controller;

use Fisharebest\Algorithm\Dijkstra;
use Fisharebest\Webtrees\Database;
use Fisharebest\Webtrees\Family;
use Fisharebest\Webtrees\GedcomRecord;
use Fisharebest\Webtrees\Individual;

/**
 * Controller for the relationships calculations
 */
class RelationshipController extends PageController {
	/**
	 * Calculate the shortest paths - or all paths - between two individuals.
	 *
	 * @param Individual $individual1
	 * @param Individual $individual2
	 * @param int        $recursion   How many levels of recursion to use
	 * @param boo;       $ancestor    Restrict to relationships via a common ancestor

	 *
	 * @return string[][]
	 */
	public function calculateRelationships(Individual $individual1, Individual $individual2, $recursion, $ancestor = false) {

		$rows = Database::prepare(
			"SELECT l_from, l_to FROM `##link` WHERE l_file = :tree_id AND l_type IN ('FAMS', 'FAMC')"
		)->execute(array(
			'tree_id' => $individual1->getTree()->getTreeId(),
		))->fetchAll();

		// Optionally restrict the graph to the ancestors of the individuals.
		if ($ancestor) {
			$ancestors = $this->allAncestors($individual1->getXref(), $individual2->getXref(), $individual1->getTree()->getTreeId());
			$exclude   = $this->excludeFamilies($individual1->getXref(), $individual2->getXref(), $individual1->getTree()->getTreeId());
		} else {
			$ancestors = array();
			$exclude   = array();
		}

		$graph = array();

		foreach ($rows as $row) {
			if (!$ancestors || in_array($row->l_from, $ancestors) && !in_array($row->l_to, $exclude)) {

				$graph[$row->l_from][$row->l_to] = 1;
				$graph[$row->l_to][$row->l_from] = 1;
			}
		}

		$xref1    = $individual1->getXref();
		$xref2    = $individual2->getXref();
		$dijkstra = new Dijkstra($graph);
		$paths    = $dijkstra->shortestPaths($xref1, $xref2);

		// Only process each exclusion list once;
		$excluded = array();

		$queue = array();
		foreach ($paths as $path) {
			// Insert the paths into the queue, with an exclusion list.
			$queue[] = array('path' => $path, 'exclude' => array());
			// While there are un-extended paths
			while (list(, $next) = each($queue)) {
				// For each family on the path
				for ($n = count($next['path']) - 2; $n >= 1; $n -= 2) {
					$exclude = $next['exclude'];
					if (count($exclude) >= $recursion) {
						continue;
					}
					$exclude[] = $next['path'][$n];
					sort($exclude);
					$tmp = implode('-', $exclude);
					if (in_array($tmp, $excluded)) {
						continue;
					} else {
						$excluded[] = $tmp;
					}
					// Add any new path to the queue
					foreach ($dijkstra->shortestPaths($xref1, $xref2, $exclude) as $new_path) {
						$queue[] = array('path' => $new_path, 'exclude' => $exclude);
					}
				}
			}
		}
		// Extract the paths from the queue, removing duplicates.
		$paths = array();
		foreach ($queue as $next) {
			$paths[implode('-', $next['path'])] = $next['path'];
		}

		return $paths;
	}

	/**
	 * Convert a path (list of XREFs) to an "old-style" string of relationships.
	 *
	 * Return an empty array, if privacy rules prevent us viewing any node.
	 *
	 * @param GedcomRecord[] $path Alternately Individual / Family
	 *
	 * @return string[]
	 */
	public function oldStyleRelationshipPath(array $path) {
		global $WT_TREE;
function myFunction($a, $b) {
    // Do something
}

		$spouse_codes  = array('M' => 'hus', 'F' => 'wif', 'U' => 'spo');
		$parent_codes  = array('M' => 'fat', 'F' => 'mot', 'U' => 'par');
		$child_codes   = array('M' => 'son', 'F' => 'dau', 'U' => 'chi');
		$sibling_codes = array('M' => 'bro', 'F' => 'sis', 'U' => 'sib');
		$relationships = array();

		for ($i = 1; $i < count($path); $i += 2) {
for ($i=0; $i<count($array); $i++) { // calls count() on each iteration
}

// Better
for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once
}
			$family = Family::getInstance($path[$i], $WT_TREE);
			$prev   = Individual::getInstance($path[$i - 1], $WT_TREE);
			$next   = Individual::getInstance($path[$i + 1], $WT_TREE);
			if (preg_match('/\n\d (HUSB|WIFE|CHIL) @' . $prev->getXref() . '@/', $family->getGedcom(), $match)) {
				$rel1 = $match[1];
			} else {
				return array();
			}
			if (preg_match('/\n\d (HUSB|WIFE|CHIL) @' . $next->getXref() . '@/', $family->getGedcom(), $match)) {
				$rel2 = $match[1];
			} else {
				return array();
			}
			if (($rel1 === 'HUSB' || $rel1 === 'WIFE') && ($rel2 === 'HUSB' || $rel2 === 'WIFE')) {
				$relationships[$i] = $spouse_codes[$next->getSex()];
			} elseif (($rel1 === 'HUSB' || $rel1 === 'WIFE') && $rel2 === 'CHIL') {
				$relationships[$i] = $child_codes[$next->getSex()];
			} elseif ($rel1 === 'CHIL' && ($rel2 === 'HUSB' || $rel2 === 'WIFE')) {
				$relationships[$i] = $parent_codes[$next->getSex()];
			} elseif ($rel1 === 'CHIL' && $rel2 === 'CHIL') {
				$relationships[$i] = $sibling_codes[$next->getSex()];
			}
		}

		return $relationships;
	}

	/**
	 * Find all ancestors of a list of individuals
	 *
	 * @param string $xref1
	 * @param string $xref2
	 * @param int    $tree_id
	 *
	 * @return array

	 */
	private function allAncestors($xref1, $xref2, $tree_id) {
		$ancestors = array($xref1, $xref2);

		$queue = array($xref1, $xref2);
		while (!empty($queue)) {
			$placeholders = implode(',', array_fill(0, count($queue), '?'));
			$parameters = $queue;
			$parameters[] = $tree_id;

			$parents = Database::prepare(
				"SELECT l2.l_from" .
				" FROM `##link` AS l1" .
				" JOIN `##link` AS l2 USING (l_to, l_file) " .
				" WHERE l1.l_type = 'FAMC' AND l2.l_type = 'FAMS' AND l1.l_from IN (" . $placeholders . ") AND l_file = ?"
			)->execute(
				$parameters
			)->fetchOneColumn();

			$queue = array();
			foreach ($parents as $parent) {
				if (!in_array($parent, $ancestors)) {
					$ancestors[] = $parent;
					$queue[]     = $parent;
				}
			}
		}

		return $ancestors;
	}

	/**
	 * Find all families of two individuals
	 *
	 * @param string $xref1
	 * @param string $xref2
	 * @param int    $tree_id
	 *
	 * @return array

	 */
	private function excludeFamilies($xref1, $xref2, $tree_id) {
		return Database::prepare(
			"SELECT l_to" .
			" FROM `##link` AS l1" .
			" JOIN `##link` AS l2 USING (l_type, l_to, l_file) " .
			" WHERE l_type = 'FAMS' AND l1.l_from = :spouse1 AND l2.l_from = :spouse2 AND l_file = :tree_id"
		)->execute(array(
			'spouse1' => $xref1,
			'spouse2' => $xref2,
			'tree_id' => $tree_id,
		))->fetchOneColumn();
	}
}


Push — 1.7 ( f5d842...5315e2 )

RelationshipController::calculateRelationships() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

1. Pass all data via parameters

2. Create a class that maintains your state

1		<?php
2		/**
3		* webtrees: online genealogy
4		* Copyright (C) 2016 webtrees development team
5		* This program is free software: you can redistribute it and/or modify
6		* it under the terms of the GNU General Public License as published by
7		* the Free Software Foundation, either version 3 of the License, or
8		* (at your option) any later version.
9		* This program is distributed in the hope that it will be useful,
10		* but WITHOUT ANY WARRANTY; without even the implied warranty of
11		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12		* GNU General Public License for more details.
13		* You should have received a copy of the GNU General Public License
14		* along with this program. If not, see <http://www.gnu.org/licenses/>.
15		*/
16		namespace Fisharebest\Webtrees\Controller;
17
18		use Fisharebest\Algorithm\Dijkstra;
19		use Fisharebest\Webtrees\Database;
20		use Fisharebest\Webtrees\Family;
21		use Fisharebest\Webtrees\GedcomRecord;
22		use Fisharebest\Webtrees\Individual;
23
24		/**
25		* Controller for the relationships calculations
26		*/
27		class RelationshipController extends PageController {
28		/**
29		* Calculate the shortest paths - or all paths - between two individuals.
30		*
31		* @param Individual $individual1
32		* @param Individual $individual2
33		* @param int $recursion How many levels of recursion to use
34		* @param boo; $ancestor Restrict to relationships via a common ancestor
		0 ignored issues – show Documentation introduced 2016-09-26 16:29 UTC by Report Bug Copy Issue Report The doc-type `boo;` could not be parsed: Expected "\|" or "end of type", but got ";" at position 3. (view supported doc-types) This check marks PHPDoc comments that could not be parsed by our parser. To see which comment annotations we can parse, please refer to our documentation on supported doc-types. Loading history...
35		*
36		* @return string[][]
37		*/
38		public function calculateRelationships(Individual $individual1, Individual $individual2, $recursion, $ancestor = false) {
39
40		$rows = Database::prepare(
41		"SELECT l_from, l_to FROM `##link` WHERE l_file = :tree_id AND l_type IN ('FAMS', 'FAMC')"
42		)->execute(array(
43		'tree_id' => $individual1->getTree()->getTreeId(),
44		))->fetchAll();
45
46		// Optionally restrict the graph to the ancestors of the individuals.
47		if ($ancestor) {
48		$ancestors = $this->allAncestors($individual1->getXref(), $individual2->getXref(), $individual1->getTree()->getTreeId());
49		$exclude = $this->excludeFamilies($individual1->getXref(), $individual2->getXref(), $individual1->getTree()->getTreeId());
50		} else {
51		$ancestors = array();
52		$exclude = array();
53		}
54
55		$graph = array();
56
57		foreach ($rows as $row) {
58		if (!$ancestors \|\| in_array($row->l_from, $ancestors) && !in_array($row->l_to, $exclude)) {
		0 ignored issues – show Bug Best Practice introduced 2016-09-26 16:29 UTC by Report Bug Copy Issue Report The expression `$ancestors` of type `array` is implicitly converted to a boolean; are you sure this is intended? If so, consider using `empty($expr)` instead to make it clear that you intend to check for an array without elements. This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent. Consider making the comparison explicit by using `empty(..)` or `! empty(...)` instead. Loading history...
59		$graph[$row->l_from][$row->l_to] = 1;
60		$graph[$row->l_to][$row->l_from] = 1;
61		}
62		}
63
64		$xref1 = $individual1->getXref();
65		$xref2 = $individual2->getXref();
66		$dijkstra = new Dijkstra($graph);
67		$paths = $dijkstra->shortestPaths($xref1, $xref2);
68
69		// Only process each exclusion list once;
70		$excluded = array();
71
72		$queue = array();
73		foreach ($paths as $path) {
74		// Insert the paths into the queue, with an exclusion list.
75		$queue[] = array('path' => $path, 'exclude' => array());
76		// While there are un-extended paths
77		while (list(, $next) = each($queue)) {
78		// For each family on the path
79		for ($n = count($next['path']) - 2; $n >= 1; $n -= 2) {
80		$exclude = $next['exclude'];
81		if (count($exclude) >= $recursion) {
82		continue;
83		}
84		$exclude[] = $next['path'][$n];
85		sort($exclude);
86		$tmp = implode('-', $exclude);
87		if (in_array($tmp, $excluded)) {
88		continue;
89		} else {
90		$excluded[] = $tmp;
91		}
92		// Add any new path to the queue
93		foreach ($dijkstra->shortestPaths($xref1, $xref2, $exclude) as $new_path) {
94		$queue[] = array('path' => $new_path, 'exclude' => $exclude);
95		}
96		}
97		}
98		}
99		// Extract the paths from the queue, removing duplicates.
100		$paths = array();
101		foreach ($queue as $next) {
102		$paths[implode('-', $next['path'])] = $next['path'];
103		}
104
105		return $paths;
106		}
107
108		/**
109		* Convert a path (list of XREFs) to an "old-style" string of relationships.
110		*
111		* Return an empty array, if privacy rules prevent us viewing any node.
112		*
113		* @param GedcomRecord[] $path Alternately Individual / Family
114		*
115		* @return string[]
116		*/
117		public function oldStyleRelationshipPath(array $path) {
118		global $WT_TREE;
		0 ignored issues – show Compatibility Best Practice introduced 2015-11-21 10:27 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
119
120		$spouse_codes = array('M' => 'hus', 'F' => 'wif', 'U' => 'spo');
121		$parent_codes = array('M' => 'fat', 'F' => 'mot', 'U' => 'par');
122		$child_codes = array('M' => 'son', 'F' => 'dau', 'U' => 'chi');
123		$sibling_codes = array('M' => 'bro', 'F' => 'sis', 'U' => 'sib');
124		$relationships = array();
125
126		for ($i = 1; $i < count($path); $i += 2) {
		0 ignored issues – show Performance Best Practice introduced 2015-11-21 10:27 UTC by Report Bug Copy Issue Report It seems like you are calling the size function `count()` as part of the test condition. You might want to compute the size beforehand, and not on each iteration. If the size of the collection does not change during the iteration, it is generally a good practice to compute it beforehand, and not on each iteration: for ($i=0; $i<count($array); $i++) { // calls count() on each iteration } // Better for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once } Loading history...
127		$family = Family::getInstance($path[$i], $WT_TREE);
128		$prev = Individual::getInstance($path[$i - 1], $WT_TREE);
129		$next = Individual::getInstance($path[$i + 1], $WT_TREE);
130	View Code Duplication	if (preg_match('/\n\d (HUSB\|WIFE\|CHIL) @' . $prev->getXref() . '@/', $family->getGedcom(), $match)) {
131		$rel1 = $match[1];
132		} else {
133		return array();
134		}
135	View Code Duplication	if (preg_match('/\n\d (HUSB\|WIFE\|CHIL) @' . $next->getXref() . '@/', $family->getGedcom(), $match)) {
136		$rel2 = $match[1];
137		} else {
138		return array();
139		}
140		if (($rel1 === 'HUSB' \|\| $rel1 === 'WIFE') && ($rel2 === 'HUSB' \|\| $rel2 === 'WIFE')) {
141		$relationships[$i] = $spouse_codes[$next->getSex()];
142		} elseif (($rel1 === 'HUSB' \|\| $rel1 === 'WIFE') && $rel2 === 'CHIL') {
143		$relationships[$i] = $child_codes[$next->getSex()];
144		} elseif ($rel1 === 'CHIL' && ($rel2 === 'HUSB' \|\| $rel2 === 'WIFE')) {
145		$relationships[$i] = $parent_codes[$next->getSex()];
146		} elseif ($rel1 === 'CHIL' && $rel2 === 'CHIL') {
147		$relationships[$i] = $sibling_codes[$next->getSex()];
148		}
149		}
150
151		return $relationships;
152		}
153
154		/**
155		* Find all ancestors of a list of individuals
156		*
157		* @param string $xref1
158		* @param string $xref2
159		* @param int $tree_id
160		*
161		* @return array
		0 ignored issues – show Documentation introduced 2016-09-26 16:29 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `string[]`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
162		*/
163		private function allAncestors($xref1, $xref2, $tree_id) {
164		$ancestors = array($xref1, $xref2);
165
166		$queue = array($xref1, $xref2);
167		while (!empty($queue)) {
168		$placeholders = implode(',', array_fill(0, count($queue), '?'));
169		$parameters = $queue;
170		$parameters[] = $tree_id;
171
172		$parents = Database::prepare(
173		"SELECT l2.l_from" .
174		" FROM `##link` AS l1" .
175		" JOIN `##link` AS l2 USING (l_to, l_file) " .
176		" WHERE l1.l_type = 'FAMC' AND l2.l_type = 'FAMS' AND l1.l_from IN (" . $placeholders . ") AND l_file = ?"
177		)->execute(
178		$parameters
179		)->fetchOneColumn();
180
181		$queue = array();
182		foreach ($parents as $parent) {
183		if (!in_array($parent, $ancestors)) {
184		$ancestors[] = $parent;
185		$queue[] = $parent;
186		}
187		}
188		}
189
190		return $ancestors;
191		}
192
193		/**
194		* Find all families of two individuals
195		*
196		* @param string $xref1
197		* @param string $xref2
198		* @param int $tree_id
199		*
200		* @return array
		0 ignored issues – show Documentation introduced 2016-09-26 16:29 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `string[]`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
201		*/
202		private function excludeFamilies($xref1, $xref2, $tree_id) {
203		return Database::prepare(
204		"SELECT l_to" .
205		" FROM `##link` AS l1" .
206		" JOIN `##link` AS l2 USING (l_type, l_to, l_file) " .
207		" WHERE l_type = 'FAMS' AND l1.l_from = :spouse1 AND l2.l_from = :spouse2 AND l_file = :tree_id"
208		)->execute(array(
209		'spouse1' => $xref1,
210		'spouse2' => $xref2,
211		'tree_id' => $tree_id,
212		))->fetchOneColumn();
213		}
214		}
215

fisharebest / webtrees

Push — 1.7 ( f5d842...5315e2 )

RelationshipController::calculateRelationships() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like