-
Notifications
You must be signed in to change notification settings - Fork 20
Open
Description
Some refactoring ideas to improve the library, ideas are mostly proposed to reduce code complexity for the main classes (HarvestApi and HarvestReport):
- Reduce
\Harvest\HarvestApi(maybe rename it to..\Connection?) to something which will just handle authentication and create a\Harvest\HttpClientinstance (we should be able to overload the used client class, f.e.\Harvest\Test\MockedHttpClient, which will use cached responses). - Move the methods of each API (Tasks, Clients, Projects, Time, etc.) to its own class. This will introduce a new class for each API, f.e.
\Harvest\Api\Tasks, with the methodsgetTasks,getTask,create..,update..,delete... You get the point. While we're at it we can also rename them to something a bit more concise, likelist(),get(),create(), etc. - Introduce a protected
$propertieson each Model which describes valid attributes in a XML request/response and their type (int,string,DateTime, etc.). We could do some marshaling to convert these values when reading from the API, so we return aDateTimeobject instead of a string.
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.
Metadata
Metadata
Assignees
Labels
No labels