gridonic / hapi

PHP Wrapper Library for the Harvest API
GNU General Public License v3.0
41 stars 21 forks source link

Moving forward: a TODO #11

Open bjornpost opened 9 years ago

bjornpost commented 9 years ago

Some refactoring ideas to improve the library, ideas are mostly proposed to reduce code complexity for the main classes (HarvestApi and HarvestReport):

A rough idea of how these changes could affect the public API:

<?php
    $harvest = new \Harvest\HarvestApi;
    $harvest->authenticate('user', 'pass');
    try {
        /**
         * The api() method will return a \Harvest\Api\X instance or throws
         * a \Harvest\Exception\UnknownApiException because the requested
         * API does not exist. The index() call will return the actual data.
         *
         */
        $tasks = $harvest->api('task')->index();
    } catch (\Harvest\Exception $e) {
        /**
         * A bunch of Exceptions could be thrown, like:
         *  - \Harvest\Exception\AuthenticationException
         *  - \Harvest\Exception\UnknownApiException
         *  - \Harvest\Exception\IncompleteRequestException
         *  - \Harvest\Exception\ApiLimitExceedException
         *  - ...?
         *
         */
    }

When we start moving things around, we should think about how we can make the library easy to test without requiring an internet connection. Maybe we could mock HTTP requests and responses? That way we know for sure our requests were valid at one point in time, but we won't be notified if Harvest decides to update their API (and syntax). Our tests will still pass.

bjornpost commented 9 years ago

Thoughts, @peschee?

bjornpost commented 9 years ago

Update; I've started working on this in bjornpost/hapi/rewrite.

peschee commented 9 years ago

Hey @bjornpost, sorry for my late reply. I'm currently quite busy. The steps you outlines seem totally valid to me. I'm currently not using the Harvest API anywhere in production, but I think we should think about proper testing of all the API methods.

Maybe you could have a look at using https://github.com/guzzle/guzzle as a dependency for the underlying HTTP communication. This would probably also ease the testing, since we could start using mocks http://docs.guzzlephp.org/en/latest/testing.html?highlight=mock I assume testing against the live API ist not very practical, even though yes, we won't notice when Harvest changes something. For proper internet tests, Harvest should provide a test account with example data. I'm not sure they offer that.