Skip to content

Commit 9a9373d

Browse files
committed
Iterate node lists with foreach
`DOMNodeList` implements `Traversable`. There are some `for` loops left but we cannot simply replace those: PHP follows the DOM specification, which requires that `NodeList` objects in the DOM are live. As a result, any operation that removes a node list member node from its parent (such as `removeChild`, `replaceChild` or `appendChild`) will cause the next node in the iterator to be skipped. We could work around that by converting those node lists to static arrays using `iterator_to_array` but not sure if it is worth it.
1 parent d454c3a commit 9a9373d

File tree

1 file changed

+14
-14
lines changed

1 file changed

+14
-14
lines changed

src/Readability.php

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,7 @@ public function addFootnotes(\DOMElement $articleContent): void
302302
$articleLinks = $articleContent->getElementsByTagName('a');
303303
$linkCount = 0;
304304

305-
for ($i = 0; $i < $articleLinks->length; ++$i) {
306-
$articleLink = $articleLinks->item($i);
305+
foreach ($articleLinks as $articleLink) {
307306
$footnoteLink = $articleLink->cloneNode(true);
308307
$refLink = $this->dom->createElement('a');
309308
$footnote = $this->dom->createElement('li');
@@ -383,8 +382,8 @@ public function prepArticle(\DOMNode $articleContent): void
383382

384383
// Remove service data-candidate attribute.
385384
$elems = $xpath->query('.//*[@data-candidate]', $articleContent);
386-
for ($i = $elems->length - 1; $i >= 0; --$i) {
387-
$elems->item($i)->removeAttribute('data-candidate');
385+
foreach ($elems as $elem) {
386+
$elem->removeAttribute('data-candidate');
388387
}
389388

390389
// Clean out junk from the article content.
@@ -520,11 +519,12 @@ public function getLinkDensity(\DOMElement $e, bool $excludeExternal = false): f
520519
$textLength = mb_strlen($this->getInnerText($e, true, true));
521520
$linkLength = 0;
522521

523-
for ($dRe = $this->domainRegExp, $i = 0, $il = $links->length; $i < $il; ++$i) {
524-
if ($excludeExternal && $dRe && !preg_match($dRe, $links->item($i)->getAttribute('href'))) {
522+
$dRe = $this->domainRegExp;
523+
foreach ($links as $link) {
524+
if ($excludeExternal && $dRe && !preg_match($dRe, $link->getAttribute('href'))) {
525525
continue;
526526
}
527-
$linkLength += mb_strlen($this->getInnerText($links->item($i)));
527+
$linkLength += mb_strlen($this->getInnerText($link));
528528
}
529529

530530
if ($textLength > 0 && $linkLength > 0) {
@@ -640,15 +640,15 @@ public function cleanConditionally(\DOMElement $e, string $tag): void
640640
$embedCount = 0;
641641
$embeds = $node->getElementsByTagName('embed');
642642

643-
for ($ei = 0, $il = $embeds->length; $ei < $il; ++$ei) {
644-
if (preg_match($this->regexps['media'], $embeds->item($ei)->getAttribute('src'))) {
643+
foreach ($embeds as $embed) {
644+
if (preg_match($this->regexps['media'], $embed->getAttribute('src'))) {
645645
++$embedCount;
646646
}
647647
}
648648

649649
$embeds = $node->getElementsByTagName('iframe');
650-
for ($ei = 0, $il = $embeds->length; $ei < $il; ++$ei) {
651-
if (preg_match($this->regexps['media'], $embeds->item($ei)->getAttribute('src'))) {
650+
foreach ($embeds as $embed) {
651+
if (preg_match($this->regexps['media'], $embed->getAttribute('src'))) {
652652
++$embedCount;
653653
}
654654
}
@@ -1018,15 +1018,15 @@ protected function grabArticle(?\DOMElement $page = null)
10181018
* A score is determined by things like number of commas, class names, etc.
10191019
* Maybe eventually link density.
10201020
*/
1021-
for ($pt = 0, $scored = \count($nodesToScore); $pt < $scored; ++$pt) {
1022-
$ancestors = $this->getAncestors($nodesToScore[$pt], 5);
1021+
foreach ($nodesToScore as $nodeToScore) {
1022+
$ancestors = $this->getAncestors($nodeToScore, 5);
10231023

10241024
// No parent node? Move on...
10251025
if (0 === \count($ancestors)) {
10261026
continue;
10271027
}
10281028

1029-
$innerText = $this->getInnerText($nodesToScore[$pt]);
1029+
$innerText = $this->getInnerText($nodeToScore);
10301030

10311031
// If this paragraph is less than MIN_PARAGRAPH_LENGTH (default:20) characters, don't even count it.
10321032
if (mb_strlen($innerText) < self::MIN_PARAGRAPH_LENGTH) {

0 commit comments

Comments
 (0)