REST-API 3rd party resource model class

Question

I'm working on an integration for Magento 2 with a 3rd party. This class will fetch the articledata resource. This resource contains multiple items, with a maximum of 750 per call. The class is designed to be used like do load() while next().

If there are more items than the current 750, there will be a value next_resultset. For example /app/api/v3/articledata/750-1-36/. I only need the last part 750-1-36/ to append it to the resource.

This class works fine, but I'm not happy with how all the logic is being handled. Part of me tells that the API I'm using is not designed so well and another part of me tells that the code could be better regardless.

<?php

class ArticleData
{
    const RESULT_SET_LIMIT = 750;

    /**
     * @var RestClient
     */
    protected $client;

    /**
     * @var ConfigProviderFactory
     */
    protected $configProviderFactory;

    /**
     * @var array
     */
    protected $data = [];

    /**
     * @var \Magento\Framework\Json\Decoder
     */
    protected $jsonDecoder;

    /**
     * ArticleData constructor.
     * @param ConfigProviderFactory $configProviderFactory
     * @param RestClient $client
     * @param \Magento\Framework\Json\Decoder $jsonDecoder
     */
    public function __construct(
        ConfigProviderFactory $configProviderFactory,
        RestClient $client,
        \Magento\Framework\Json\Decoder $jsonDecoder
    ) {
        $this->configProviderFactory = $configProviderFactory;
        $this->client = $client;
        $this->jsonDecoder = $jsonDecoder;
    }

    /**
     * Load first set
     */
    public function load()
    {
        $resultSet = '';
        if ($this->next()) {
            $resultSet = $this->getResultSet();
        }

        $response = $this->client->get('articledata/' . $resultSet);

        $decoded = $this->jsonDecoder->decode($response->getBody());
        $decoded['items'] = $decoded['data'];
        unset($decoded['data']);

        $this->data = $decoded;
    }

    /**
     * @return bool
     */
    public function next(): bool
    {
        return !empty($this->data['results']) && $this->data['results'] >= self::RESULT_SET_LIMIT
            && !empty($this->data['next_resultset']);
    }

    /**
     * @return string
     */
    public function getResultSet(): string
    {
        $matches = [];
        return preg_match('/articledata\/([0-9\-]+\/)/', $this->data['next_resultset'], $matches) ? $matches[1] : '';
    }
}

Answer 1

• Where is your namespace? If your are working in code bases that use namespaces, like Magento, you really should be namespacing your code as well.
• The interface presented seems confusing to me. Is it your expectation that the caller controls pagination through the result set? If so, then how does the caller actually work with intermediate result sets, given that there is no interface to return the data set to the caller and your data in $this->data are overwritten with every iteration of load()? How is caller supposed to assemble the results? Also I would expect method names like getResultSet() to, you know, actually return a result set to the caller, not a string format used for pagination. Why would this method need a public interface anyway?
• I think you need make the determination as to whether this class should abstract away pagination activity from caller or not. If you want the caller to have full control of pagination (and potentially assembling result sets based on pagination results), then I would think your class would need an interface that a) is more explicit around result set size and page numbers to be used b) actually return result set data to the caller. This class likely does not need to maintain state the way you do now if you want caller to own pagination control.
• You have a data mapping step in your load() function, where you change key for data set from data to items. I would suggest that this mapping function should exist in a separate method. In the class, so you can keep you mapping logic separate from your loading logic.