3
\$\begingroup\$

I'm building an API wrapper for a bookkeeping SOAP API.

I have some questions regarding bast practice for structure of the wrapper and for error handling.

For now i've structured it like this:

EconClient.php

class EconClient {

    public function __construct() 
    {
        $wsdlUrl = 'https://api.e-conomic.com/secure/api1/EconomicWebservice.asmx?WSDL';
        $this->client = new SoapClient($wsdlUrl, array("trace" => 1, "exceptions" => 1));                    
    }

    public function connectWithToken($token, $appToken)
    {
        $this->client->ConnectWithToken(array('token' => $token, 'appToken' => $appToken));
    }

    public function debtor()
    {   
        $this->debtor = new Debtor($this->client);
        return $this->debtor;
    }

    public function debtorGroup()
    {
        $this->debtorGroup = new DebtorGroup($this->client);
        return $this->debtorGroup;
    }
}

Debtor.php

class Debtor {

    public function __construct($client)
    {
        $this->client = $client;
    }

    public function get($param, $value)
    {
        return $this->client->Debtor_GetData(array('entityHandle' => $this->getHandle($param, $value)))->Debtor_GetDataResult;
    }

    public function getHandle($param, $value)
    {
        switch ($param) {
            case 'number':
                return $this->client->Debtor_FindByNumber(array('number' => $value))->Debtor_FindByNumberResult;
                break;
            case 'email':
                return $this->client->Debtor_FindByEmail(array('number' => $value))->Debtor_FindByEmailResult;
    }
}

Is there any way i could structure this so i would'nt have to pass in the $client into the resource classes?

The only response i can get from the SOAP client except from data in case of success is exceptions. If i'm trying to fetch an ID that doesn't exist i will get an exception too. Should i try and catch these exceptions in for example Debtor.php? Or how could i structure error handling?

\$\endgroup\$
4
\$\begingroup\$

First off:

The only response i can get from the SOAP client except from data in case of success is exceptions.

Of course, if your call wasn't successful, there's something wrong, and an exception should be thrown. If you query for an ID that doesn't exist, don't catch the exception. That means the user has provided your client with invalid data, and its that user who must deal with the problem. Not You.
An API doesn't really deal with that many exceptions. In fact, I'm one of those people who think that a well written, bug free API-wrapper doesn't need exception handling. If the wrapper/client's code is properly put together, exceptions are the result of either bad usage or exceptions being returned by the webservice. Both types of errors should be dealt with by the user.

If you don't want to pass the client to all those classes, the answer is simple: inheritance:

class BaseClient
{
    protected $client = null;//declare your properties!!!
    protected $config = null;
    //use type hints, default = null means you don't HAVE to pass the argument
    public function __construct(\SoapClient $client = null)
    {
        $this->client = $client;
    }
    //lazy-loading getter
    protected function getClient()
    {
        if ($this->client === null)
        {//set client only when it's required
            $this->setClient(
                new \SoapClient($this->config['wsdl'], $config['options']
            );
        }
        return $this->client;
    }
    //public to allow injection
    protected function setClient(\SoapClient $client)
    {
        $this->client = $client;
        return $this;//makes your api chainable
    }
}

Then define all the classes you'll actually be using along these lines:

class DebptorClient extends BaseClient
{
    protected $config = array(//optional
        'wsdl' => 'the specific wsdl',
        'options'=> array()//defaults
    );

    public function get($param, $value)
    {
        $client = $this->getClient();//loads if not yet loaded
        return $client->Debtor_GetData(
            array(
                'entityHandle' => $this->getHandle($param, $value)
            )
        )->Debtor_GetDataResult;
    }
}

And so on.

Now, some actual code-review:

Declare your properties

From the wrapper tag wiki:

A wrapper is an OOP technique where an object encapsulates (wraps) another object, hiding/protecting the object and controlling all access to it.

By not declaring $this->client, client is effectively added later on in your objects life, which means property lookups will be slower, but more importantly: $this->client will be a public property. If $this->client is public, then you don't have a wrapper because:

A wrapper [...] encapsulates another object, hiding/protecting the object and controlling all access to it.

Thus, $this->client has to be protected or private.
Next.

Seeing this code worries me:

public function getHandle($param, $value)
{
    switch ($param) {
        case 'number':
            return $this->client->Debtor_FindByNumber(array('number' => $value))->Debtor_FindByNumberResult;
            break;//unreachable statement, btw
        case 'email':
            return $this->client->Debtor_FindByEmail(array('number' => $value))->Debtor_FindByEmailResult;
}

You're expecting the people who are to use your api to know that $param and $value are expected to be. $param has only 2 valid options: number or email. anything else is invalid, yet at no point to you bother to validate the data you're being passed.
When developing an API wrapper, it's not a bad idea to create Argument models:

namespace Soap\Data;
class Argument
{
    private $type = null;
    private $value = null;
    public function __construct($type, $value)
    {
        $setter = 'set'.ucfirst(trim($type));//email becomes setEmail
        if (!method_exists($setter, $this))
        {
            throw new \InvalidArgumentException($type.' is not a valid type for '.__CLASS__);
        }
        $this->{$setter}($value);
    }
    public function setEmail($email)
    {
        if (!filter_var($email, \FILTER_VALIDATE_EMAIL))
        {
            throw new \InvalidArgumentException($email.' is not a valid email address');
        }
        $this->value = $email;
        return $this;
    }
}

This allows you to define methods like this:

public function getByType(Argument $value)

and the Argument class ensures that the data you'll receive is validated properly.

Now, error handling.
That very much depends on how you see your code being used. Is it supposed to be a sort of "module" to be included into various projects, in which case, I'd just throw my exceptions out for the caller to deal with them. Code that is used as a dependency shouldn't have to anticipate all sorts of errors that might occur, that's the users job.

If you want this code to work sort of in the background, You could write your own exceptions (extending from either the base Exception class, or SoapFault) if you wanted to.
You could then register your own exception/error handler and unset/restore the old at lib, and quietly log the exceptions. The user, who will then be left clueless as to what actually went wrong will hate you for this. Having to check the logs constantly is a pain when developing, so add some manual overrides, too

class BaseClient
{
    protected static $exceptions = false;
    public function __construct()
    {
        if (static::$exceptions === false)
        {//static to avoid setting handler more than once
            static::$exceptions = true;
            set_exception_handler(array($this, 'exceptionHandler'));
    }
    public function exceptionHandler(\Exception $e)
    {
        //handle
    }
    public function freeExceptions()
    {
        if (static::$exceptions)
        {
            restore_exception_handler();
            static::$exceptions = false;
        }
    }
    public function __destruct()
    {
        $this->freeExceptions();
    }
}

That's one way of going about your business. Generally, though, I use the occasional try-catch block, if I've created a method that strings together a series of Soap calls, for example, but most of the time I consider Exceptions best handled by the code that invoked the method where the exception is thrown, not by the code that throws the exception.

\$\endgroup\$
  • \$\begingroup\$ Thank you for such an awesome answer! I've got a couple of things to read up on now :) \$\endgroup\$ – Stromgren Dec 22 '13 at 12:49
1
\$\begingroup\$

As for passing in $client in your resource class I don't see a real problem there, are there any specific reason for why you do not want to do it?

...

When it comes to error handling in API's there's a few things you can do that are usually API-specific.

1) First you need to determine how the API should be used, and with that you need to determine what kind of exceptions are useful for the API consumer. There are two general guidelines that can be built upon.

  • If there is an internal exception in the API due to it being poorly written (not saying that's the case here) then you most likely want to try/catch them and let them fail silently, or fairly silently (i.e., write the exception to stderr). e.g., an IndexOutOfBoundsException or maybe a NullPointerReferenceException is thrown...
  • If an error is thrown because the API consumer are using it wrongly, or a service that the API depends on doesn't work (maybe https://api.e-conomic.com/secure/api1/EconomicWebservice.asmx?WSDL is down), then let them bubble up to the API consumer and let him handle the exceptions.

2) Then of course there are cases which fall inbetween these two points and cases that are derivative or in the outskirts of the cases - in that case it's best if you as the API developer take stand on what to do.

If you can't decide what to do it's often best to let the API consumer handle the exceptions instead of just failing silently.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.