JSON API for some bank account

Question

I have been developing this class, and was wondering if anyone had any thoughts on how I can improve the performance of it.

<?php
class Something {
    private $APIUsername, $APIPassword;
    private $APIurl = 'somesite.com';

    function __construct ($APIUsername = '', $APIPassword = '') {
        try {
            if (!$APIUsername || !$APIPassword) {
                throw new Exception('You must specify a valid API username and password.');
            }

            $this->APIUsername = $APIUsername;
            $this->APIPassword = $APIPassword;
        } catch (Exception $e) {
            die($e->getMessage());
        }
    }

    /*
    * Authenticate Account
    */
    public function authenticate ($EmailAddress, $Password) {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;

            $data = $this->setJSON($data);

            $result = $this->fetch('authenticate', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Get Contacts
    */
    public function contacts ($EmailAddress, $Password, $PIN) {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;
            $data['PIN'] = $PIN;

            $data = $this->setJSON($data);

            $result = $this->fetch('contacts', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Get Transactions
    */
    public function transactions ($EmailAddress, $Password, $PIN) {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;
            $data['PIN'] = $PIN;

            $data = $this->setJSON($data);

            $result = $this->fetch('transactions', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Validate PIN
    */
    public function validatepin ($EmailAddress, $Password, $PIN) {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;
            $data['PIN'] = $PIN;

            $data = $this->setJSON($data);

            $result = $this->fetch('validatepin', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Get Account Balance
    */
    public function balance ($EmailAddress, $Password) {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;

            $data = $this->setJSON($data);

            $result = $this->fetch('balance', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Get Account Information
    */
    public function account_information ($EmailAddress, $Password) {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;

            $data = $this->setJSON($data);

            $result = $this->fetch('account_information', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Send Money
    */
    public function send ($EmailAddress, $Password, $PIN, $DestinationID, $Amount, $Notes = '', $FundsSource = '') {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;
            $data['PIN'] = $PIN;
            $data['DestinationID'] = $DestinationID;
            $data['Amount'] = $Amount;
            $data['Notes'] = urlencode($Notes);
            $data['FundsSource'] = $FundsSource;

            $data = $this->setJSON($data);

            $result = $this->fetch('send', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Send (Sender Assumes Transaction Costs)
    */
    public function send_assume_costs ($EmailAddress, $Password, $PIN, $DestinationID, $Amount, $Description = '') {
        try {
            $data['EmailAddress'] = $EmailAddress;
            $data['Password'] = $Password;
            $data['PIN'] = $PIN;
            $data['DestinationID'] = $DestinationID;
            $data['Amount'] = $Amount;
            $data['Description'] = urlencode($Description);

            $data = $this->setJSON($data);

            $result = $this->fetch('send_assume_costs', $data);
        } catch (Exception $e) {
            $result = $e->getMessage();
        }

        return $result;
    }

    /*
    * Set JSON Data
    */
    private function setJSON ($data) {
        $a = array();

        $a['APIUsername'] = $this->APIUsername;
        $a['APIPassword'] = $this->APIPassword;

        foreach ($data as $key => $value) {
            $a[$key] = $value;
        }

        return json_encode($a);
    }

    /*
    * Helper method that talks to teh API
    */
    private function fetch($APIMethod, $data) {
        $c = curl_init($this->APIurl.$APIMethod);
        curl_setopt($c, CURLOPT_SSL_VERIFYPEER, false);
        curl_setopt($c, CURLOPT_RETURNTRANSFER, 1);
        curl_setopt($c, CURLOPT_HTTPHEADER, array('Accept: application/json', 'Content-Type: application/json'));
        curl_setopt($c, CURLOPT_POSTFIELDS, $data);

        $returned = curl_exec($c);
        if ($returned === false) {
            throw new Exception(curl_error($c)); return;
        }

        curl_close($c);

        if (json_decode($returned)) {
            return $returned;
        } else {
            throw new Exception('Invalid Service Request.'); return;
        }
    }
}
?>

Answer 1

Some things I noticed.

It seems like a lot/all of your public functions are simply doing the same thing, setting the functions params into an array, calling setJSON which simply appends the APIusername and APIpassword and then you fetch and catch any errors. You could have one method that does this and only methods that differ from this logic would do something else.

looks like you can just use PHP's compact() function for most of what you are doing.

For setJSON(), instead of looping through $data and remaking the same array, why not just add $data['APIUsername'] = $this->APIUsername; and $data['APIPassword'] = $this->APIPassword; and then return the json_encode($data)

For your fetch() method, you could get rid of the curl requirement and just use streams in PHP using a combination of stream_context_create() and file_get_contents().

I think it also might be useful to glance over this articles about Exceptions .

If you really are just passing along data with the username and password attached, you may just want to just use the magic __call method, that will check an array of acceptable API methods to call and then take the params, compact, add username and password, and then call your fetch method with this data and return. Example below.

<?php
class API {
    protected $_api_methods = array(
        'balance' => array('EmailAddress', 'Password'),
    );

    private $_api_username;
        private $_api_password;
    private $_api_url = 'someurl.com';

    public function __construct($username, $password)
    {
        if (!$username || !$password) {
            throw new APIException('You must provide API credentials');
        }

        $this->_api_username = $username;
        $this->_api_password = $password;
    }

    public function __call($method_name, $arguments)
    {
        if (!in_array($method_name, array_keys($this->_api_methods)) {
            throw new APIException("$method_name is not a valid API method");
        }

        $data = $this->setJSON($method_name, $arguments);
        return $this->fetch($method_name, $data);
    }

    protected function setJSON($method_name, $data)
    {
        if (empty($data)) {
            throw new APIException('No data provided');
        }

        $data = array_combine($this->_api_methods[$method_name], $data);

        $data['api_username'] = $this->_api_username;
        $data['api_password'] = $this->_api_password;

        return json_encode($data);
    }

    protected function fetch($method, $data)
    {
        $context = stream_context_create(array(
            'http' => array(
              'method'  => 'GET',
              'timeout' => 5,
            ),
        ));

        try {
            $ret = file_get_contents($this->_api_url . $method, false, $context); 
        } catch (APIException $e) {
                $ret = $e->getMessage();
        }

        return $ret;
    }
}

class APIException extends Exception {}

Answer 2

I think with "performance" you mean the cost of keeping a PHP session open as long as the CURL request waits for an answer. If it is about "speed in PHP" or "loading time" etc. then please don't get me wrong, but PHP is a farm horse, it never will make a good racehorse.

Technically there is no way to speed up remote API calls, as you always need to wait for that. The CURL library should usually performs very well, however I am not an expert when it comes to PHP+CURL.

The only thing I spot is that you disable SSL certificate checking. Doing that allows for MITM attacks, so perhaps this is not a good idea (SSL without verify is like a condom with a hole).

What comes into my mind is that it always is not a good idea to keep RPC calls with a longer duration around, so I would recommend to change that into more phases. However that voids your class, as the complete framework has to be rewritten.

I cannot show you code, but here is the idea:

1. When the user submits the RPC to your app, the app stores the request (into some database table) and returns immediately, so the user can see something like "processing" on the page.

2. In the background some task picks up the request which is stored (in the database) and processes it. The result then is saved (in the database).

3. The client side then polls (or waits, retries, whatever) until the result is available.

The "funny" thing about this is, that this takes even longer because of the handling of the background job. However as the user can see some progress the waiting experience is better.

Perhaps you can even do it completely asynchronous, so if the user transfers money you see some "scheduled tasks" list, so the user can continue to do other things (this probably involves Ajax). And then, when the result is available, the "scheduled task" blinks (or whatever, you get the idea) such that the user can look into the result.

All is a design issue: Think about a DoS situation, where 1000 users click onto the "send money" button in the same second (perhaps after some TV add ran). Your web server then needs to keep open 1000 sessions until all the 1000 CURL requests are done in parallel. The API side usually will have some DoS prevention and reject 980 CURL requests, because you are only allowed to have 20 requests open simultaneously with the same API key (this is quite common).

So you have to serialize the requests anyway. Doing this from within PHP (like from within your class) is a pain in the a** (starvation problems, aborting manually. debugging concurrency, etc.), but with the background tasks it is easy: If you only have, say, 10 concurrent processors of stored requests, there never will be any problem with reaching the API request limit.