CRAP index (56) in Web Scraper Engine

Question

I am working on a Web Scraper for the first time using Test Driven Development, however I have caught myself into a huge CRAP (Change Risk Anti-Patterns) index (56) and I can not seem to find a solution about this.

<?php

/**
 * Code Snippet MIT licensed taken from proprietary source.
 */

namespace WebScraper;
use WebScraper\Contracts\Scraper;
use WebScraper\Collections\BaseCollection;
use WebScraper\Exceptions\EngineIgnitionException;

class Engine
{
    protected $targetAdAge;

    protected $currentAdAge = 1;

    protected $targetPage  = 999;

    protected $currentPage = 1;

    public function __construct(Scraper $scraper, BaseCollection $collection)
    {
        $this->scraper    = $scraper;
        $this->collection = $collection;
    }

    public function start($targetAdAge = null)
    {
        if ($this->collection->count() >= 1)
        {
            throw new EngineIgnitionException("Engine can not be ran twice in the same instance.");
        }

        if (array_key_exists('adage', $requestData = $scraper->queryBuilder()->buildArray()))
        {
            $this->targetAdAge = $requestData['adage'];
        }

        if (! $this->targetAdAge)
        {
            throw new EngineIgnitionException("targetAdAge value is a core part of the Engine and it is missing.");
        }

        for ($this->currentAdAge = 1; $this->currentAdAge <= $this->targetAdAge; ++$this->currentAdAge)
        {
            for ($this->currentPage = 1; $this->currentPage <= $this->targetPage; ++$this->currentPage)
            {
                $this->scraper->queryBuilder()->adage($this->currentAdAge)->page($this->currentPage);
                $parser = $this->scraper->extract()->parse();
                $items = $parser->items();
                $this->targetPage = $parser->maxPages();
                foreach ($items as &$item)
                {
                    $item->age = (new DateTime('now'))->modify("-{$this->currentAdAge} day");
                    $this->collection->push($item);
                }

                unset($item);
            }
        }

        return $this->collection;
    }
}

The main issue is in the start() function.

Basically, what this class does is building an Engine to web scrape a website that uses pagination to show their data, and because of so I am forced to use iterators and scrape every single page to gather all the data. Line 48 is the function that does the dirty job and scrapes the website.

The thing is, this function is likely to take even minutes to complete if there's a lot of data to extract or if scraping filters are very broad. This is the only function where I really do not know how to refactor it in a better way. The good thing, however, is that every other single class has a low CRAP index (< 5) and Code Coverage is above 85%.

How do I refactor the iterator of page and adAge?

Answer 1

The code looks mostly fine to me. If you feel that the code isn't as clean as it should be, or just want it to pass the metric, what you could do is split it into functions:

            $this->scraper->queryBuilder()->adage($this->currentAdAge)->page($this->currentPage);
            $parser = $this->scraper->extract()->parse();
            $items = $parser->items();
            $this->targetPage = $parser->maxPages();

That goes into a private function which sets targetPage and returns $items... you could call it parsePageToItems

And this

            foreach ($items as &$item)
            {
                $item->age = (new DateTime('now'))->modify("-{$this->currentAdAge} day");
                $this->collection->push($item);
            }

            unset($item);

Could go into a separate function mapItems.

You could then write a test for mapItems in particular, increasing coverage and further reducing your CRAP score.