PathIterator::tryGotoNextIterationElement() - Code Metrics - Inspection of "Ftre filter" - SuRaMoN/simplexmlreader - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#7)

unknown

created 2017-08-11 06:26 UTC

PathIterator::tryGotoNextIterationElement() C

↳ Parent: PathIterator

Complexity

Conditions	22
Paths	106

Size

Total Lines	61
Code Lines	33

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
c	1
b	0
f	0
dl	0
loc	61
rs	5.9311
cc	22
eloc	33
nc	106
nop	0

How to fix Long Method Complexity

<?php

namespace SimpleXmlReader;

use XMLReader;
use Iterator;
use DOMDocument;

class PathIterator implements Iterator
{
    const IS_MATCH = 'IS_MATCH';
    const DESCENDANTS_COULD_MATCH = 'DESCENDANTS_COULD_MATCH';
    const DESCENDANTS_CANT_MATCH = 'DESCENDANTS_CANT_MATCH';

    /*
     * The list of return codes for filtering callback function
     */
    /*
     * Valid elem, no filtering.
     */
    const ELEMENT_IS_VALID = 1; // elem
    /*
     * Invalid elem and its descendants, so have to be filtered out.
     */
    const ELEMENT_IS_INVALID = 2;
    /*
     * The same as `ELEMENT_IS_INVALID`. Additionaly after it sibling elems(and its descendants) have to be filtered out too.
     */
    const SIBLINGS_ARE_INVALID = 3;

    protected $reader;
    protected $searchPath;
    protected $searchCrumbs;
    protected $crumbs;
    protected $currentDomExpansion;
    protected $rewindCount;
    protected $isValid;
    protected $returnType;

    /*
     * Filtering callback function
     */
    protected $callback;

    public function __construct(ExceptionThrowingXMLReader $reader, $path, $returnType, $callback = null)
    {
        $this->reader = $reader;
        $this->searchPath = $path;
        $this->searchCrumbs = explode('/', $path);
        $this->crumbs = array();
        $this->matchCount = -1;
class MyClass { }

$x = new MyClass();
$x->foo = true;
        $this->rewindCount = 0;
        $this->isValid = false;
        $this->returnType = $returnType;
        $this->callback = $callback;
    }

    public function current()
    {
        return $this->currentDomExpansion;
    }

    public function key()
    {
        return $this->matchCount;
    }

    public function next()
    {
        $this->isValid = $this->tryGotoNextIterationElement();

        if ($this->isValid) {
            $this->matchCount += 1;
            $this->currentDomExpansion = $this->getXMLObject();
        }
    }

    public function rewind()
    {
        $this->rewindCount += 1;
        if ($this->rewindCount > 1) {
            throw new XmlException('Multiple rewinds not supported');
        }
        $this->next();
    }

    public function valid()
    {
        return $this->isValid;
    }

    protected function getXMLObject()
    {
        switch ($this->returnType) {
            case SimpleXMLReader::RETURN_DOM:
                return $this->reader->expand();

            case SimpleXMLReader::RETURN_INNER_XML_STRING:
                return $this->reader->readInnerXML();

            case SimpleXMLReader::RETURN_OUTER_XML_STRING:
                return $this->reader->readOuterXML();

            case SimpleXMLReader::RETURN_SIMPLE_XML:
                $simplexml = simplexml_import_dom($this->reader->expand(new DOMDocument('1.0')));
                if (false === $simplexml) {
                    throw new XMlException('Failed to create a SimpleXMLElement from the current XML node (invalid XML?)');
                }

                return $simplexml;

            default:
                throw new Exception(sprintf("Unknown return type: %s", $this->returnType));
        }
    }

    protected function pathIsMatching()
    {
        if (count($this->crumbs) > count($this->searchCrumbs)) {
            return self::DESCENDANTS_CANT_MATCH;
        }
        foreach ($this->crumbs as $i => $crumb) {
            $searchCrumb = $this->searchCrumbs[$i];
            if ($searchCrumb == $crumb || $searchCrumb == '*') {
                continue;
            }
            return self::DESCENDANTS_CANT_MATCH;
        }
        if (count($this->crumbs) == count($this->searchCrumbs)) {
            return self::IS_MATCH;
        }
        return self::DESCENDANTS_COULD_MATCH;
    }

    protected function searchForOpenTag(XMLReader $r)
    {
        // search for open tag
        while ($r->nodeType != XMLReader::ELEMENT) {
            if (! $r->tryRead()) { return false; }
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
        }
        return true;
    }

    public function tryGotoNextIterationElement()
    {
        $r = $this->reader;

        if ($r->nodeType == XMLReader::NONE) {
            // first time we do a read from the xml
            if (! $r->tryRead()) { return false; }
        } else {
            // if we have already had a match
            if (! $r->tryNext()) { return false; }
        }

        while (true) {
            // search for open tag
            if (! $this->searchForOpenTag($r)) { return false; }

            // fill crumbs
            array_splice($this->crumbs, $r->depth, count($this->crumbs), array($r->name));

            $matching = $this->pathIsMatching();

            $uf = self::ELEMENT_IS_VALID;
            if ($this->callback && is_callable($this->callback)
                && ($uf = call_user_func_array($this->callback, array($r, $this->crumbs))) !== self::ELEMENT_IS_VALID) {

                // extra check for sanity of a value returned by the user filter
                if ($uf !== self::SIBLINGS_ARE_INVALID && $uf !== self::ELEMENT_IS_INVALID ) {
                    $uf = self::ELEMENT_IS_INVALID;
                }

                $df = $r->depth;

                if ($uf === self::SIBLINGS_ARE_INVALID) { $df--; }
                $matching = self::DESCENDANTS_CANT_MATCH;
            }

            switch ($matching) {

                case self::DESCENDANTS_COULD_MATCH:
                    if (! $r->tryRead()) { return false; }
                    continue 2;

                case self::DESCENDANTS_CANT_MATCH:

                    if (! $r->tryNext()) { return false; }
                    if ($uf !== self::ELEMENT_IS_VALID) {
                        if (! $this->searchForOpenTag($r)) { return false; }
                        while ($r->depth > $df) {
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
                            if (! $r->tryNext()) { return false; }
                            if (! $this->searchForOpenTag($r)) { return false; }
                        }
                    }
                    continue 2;

                case self::IS_MATCH:
                    return true;
            }

            return false;
        }
    }
}


Pull Request — master (#7)

PathIterator::tryGotoNextIterationElement() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Available Fixes

1			<?php
2
3			namespace SimpleXmlReader;
4
5			use XMLReader;
6			use Iterator;
7			use DOMDocument;
8
9			class PathIterator implements Iterator
10			{
11			const IS_MATCH = 'IS_MATCH';
12			const DESCENDANTS_COULD_MATCH = 'DESCENDANTS_COULD_MATCH';
13			const DESCENDANTS_CANT_MATCH = 'DESCENDANTS_CANT_MATCH';
14
15			/*
16			* The list of return codes for filtering callback function
17			*/
18			/*
19			* Valid elem, no filtering.
20			*/
21			const ELEMENT_IS_VALID = 1; // elem
22			/*
23			* Invalid elem and its descendants, so have to be filtered out.
24			*/
25			const ELEMENT_IS_INVALID = 2;
26			/*
27			* The same as `ELEMENT_IS_INVALID`. Additionaly after it sibling elems(and its descendants) have to be filtered out too.
28			*/
29			const SIBLINGS_ARE_INVALID = 3;
30
31			protected $reader;
32			protected $searchPath;
33			protected $searchCrumbs;
34			protected $crumbs;
35			protected $currentDomExpansion;
36			protected $rewindCount;
37			protected $isValid;
38			protected $returnType;
39
40			/*
41			* Filtering callback function
42			*/
43			protected $callback;
44
45			public function __construct(ExceptionThrowingXMLReader $reader, $path, $returnType, $callback = null)
46			{
47			$this->reader = $reader;
48			$this->searchPath = $path;
49			$this->searchCrumbs = explode('/', $path);
50			$this->crumbs = array();
51			$this->matchCount = -1;
			0 ignored issues – show Bug introduced 2016-07-13 11:06 UTC by Report Bug Copy Issue Report The property `matchCount` does not exist. Did you maybe forget to declare it? In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code: class MyClass { } $x = new MyClass(); $x->foo = true; Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: class MyClass { public $foo; } $x = new MyClass(); $x->foo = true; Loading history...
52			$this->rewindCount = 0;
53			$this->isValid = false;
54			$this->returnType = $returnType;
55			$this->callback = $callback;
56			}
57
58			public function current()
59			{
60			return $this->currentDomExpansion;
61			}
62
63			public function key()
64			{
65			return $this->matchCount;
66			}
67
68			public function next()
69			{
70			$this->isValid = $this->tryGotoNextIterationElement();
71
72			if ($this->isValid) {
73			$this->matchCount += 1;
74			$this->currentDomExpansion = $this->getXMLObject();
75			}
76			}
77
78			public function rewind()
79			{
80			$this->rewindCount += 1;
81			if ($this->rewindCount > 1) {
82			throw new XmlException('Multiple rewinds not supported');
83			}
84			$this->next();
85			}
86
87			public function valid()
88			{
89			return $this->isValid;
90			}
91
92			protected function getXMLObject()
93			{
94			switch ($this->returnType) {
95			case SimpleXMLReader::RETURN_DOM:
96			return $this->reader->expand();
97
98			case SimpleXMLReader::RETURN_INNER_XML_STRING:
99			return $this->reader->readInnerXML();
100
101			case SimpleXMLReader::RETURN_OUTER_XML_STRING:
102			return $this->reader->readOuterXML();
103
104			case SimpleXMLReader::RETURN_SIMPLE_XML:
105			$simplexml = simplexml_import_dom($this->reader->expand(new DOMDocument('1.0')));
106			if (false === $simplexml) {
107			throw new XMlException('Failed to create a SimpleXMLElement from the current XML node (invalid XML?)');
108			}
109
110			return $simplexml;
111
112			default:
113			throw new Exception(sprintf("Unknown return type: %s", $this->returnType));
114			}
115			}
116
117			protected function pathIsMatching()
118			{
119			if (count($this->crumbs) > count($this->searchCrumbs)) {
120			return self::DESCENDANTS_CANT_MATCH;
121			}
122			foreach ($this->crumbs as $i => $crumb) {
123			$searchCrumb = $this->searchCrumbs[$i];
124			if ($searchCrumb == $crumb \|\| $searchCrumb == '*') {
125			continue;
126			}
127			return self::DESCENDANTS_CANT_MATCH;
128			}
129			if (count($this->crumbs) == count($this->searchCrumbs)) {
130			return self::IS_MATCH;
131			}
132			return self::DESCENDANTS_COULD_MATCH;
133			}
134
135			protected function searchForOpenTag(XMLReader $r)
136			{
137			// search for open tag
138			while ($r->nodeType != XMLReader::ELEMENT) {
139			if (! $r->tryRead()) { return false; }
			0 ignored issues – show Bug introduced 2017-08-09 16:40 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `XMLReader` as the method `tryRead()` does only exist in the following sub-classes of `XMLReader`: `SimpleXmlReader\ExceptionThrowingXMLReader`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
140			}
141			return true;
142			}
143
144			public function tryGotoNextIterationElement()
145			{
146			$r = $this->reader;
147
148			if ($r->nodeType == XMLReader::NONE) {
149			// first time we do a read from the xml
150			if (! $r->tryRead()) { return false; }
151			} else {
152			// if we have already had a match
153			if (! $r->tryNext()) { return false; }
154			}
155
156			while (true) {
157			// search for open tag
158			if (! $this->searchForOpenTag($r)) { return false; }
159
160			// fill crumbs
161			array_splice($this->crumbs, $r->depth, count($this->crumbs), array($r->name));
162
163			$matching = $this->pathIsMatching();
164
165			$uf = self::ELEMENT_IS_VALID;
166			if ($this->callback && is_callable($this->callback)
167			&& ($uf = call_user_func_array($this->callback, array($r, $this->crumbs))) !== self::ELEMENT_IS_VALID) {
168
169			// extra check for sanity of a value returned by the user filter
170			if ($uf !== self::SIBLINGS_ARE_INVALID && $uf !== self::ELEMENT_IS_INVALID ) {
171			$uf = self::ELEMENT_IS_INVALID;
172			}
173
174			$df = $r->depth;
175
176			if ($uf === self::SIBLINGS_ARE_INVALID) { $df--; }
177			$matching = self::DESCENDANTS_CANT_MATCH;
178			}
179
180			switch ($matching) {
181
182			case self::DESCENDANTS_COULD_MATCH:
183			if (! $r->tryRead()) { return false; }
184			continue 2;
185
186			case self::DESCENDANTS_CANT_MATCH:
187
188			if (! $r->tryNext()) { return false; }
189			if ($uf !== self::ELEMENT_IS_VALID) {
190			if (! $this->searchForOpenTag($r)) { return false; }
191			while ($r->depth > $df) {
			0 ignored issues – show Bug introduced 2017-08-09 16:40 UTC by Report Bug Copy Issue Report The variable `$df` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
192			if (! $r->tryNext()) { return false; }
193			if (! $this->searchForOpenTag($r)) { return false; }
194			}
195			}
196			continue 2;
197
198			case self::IS_MATCH:
199			return true;
200			}
201
202			return false;
203			}
204			}
205			}
206

SuRaMoN / simplexmlreader

Pull Request — master (#7)

PathIterator::tryGotoNextIterationElement() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like