Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

https://github.com/Prashles/PHP-Currency-Convert-Class

<?php

/*
*
* PHP Validation Class
* 
* The currency rates are fetched and cached for the whole day
*
* http://prash.me
* http://github.com/prashles
*
* Uses http://rate-exchange.appspot.com/currency currency API
* Returns JSON - based on Google's API
*
* @author Prash Somaiya
*
*/

Class Convert {

    /*
    * Constructor sets to TRUE if $cacheFolder is writable
    *
    * FALSE by default
    */

    private $cachable = FALSE;

    /*
    * The folder where the cache files are stored
    * 
    * Set in the constructor. //convert by default
    */

    private $cacheFolder;

    /*
    * Length of cache in seconds
    *
    * Default is 1 day
    */

    private $cacheTimeout;

    /*
    * Check if folder is writable for caching
    *
    * Set $cache to FALSE on call to disable caching
    * $folder is where the cache files will be stored
    *
    * Set $folder to 'dcf' for the default folder
    *
    * Set $cacheTimeout for length of caching in seconds
    */

    public function __construct($cache = TRUE, $folder = 'dcf', $cacheTimeout = 86400)
    {
        $this->cacheFolder = ($folder == 'dcf') ? dirname(__FILE__).'/convert/' : $folder;

        if (is_writable($this->cacheFolder) && $cache == TRUE) {        
            $this->cachable     = TRUE;
            $this->cacheTimeout = $cacheTimeout;        
        }
    }

    /*
    * Main function for converting
    *
    * Set $round to FALSE to return full amount
    */

    public function convert($amount = 1, $from = 'GBP', $to = 'USD', $round = TRUE)
    {

        # Check if cache file exists and pull rate

        $rate = $this->get_cache($from.$to);

        if ($rate !== FALSE) {          
            $return = $rate * $amount;          
        }

        else {

            if (!$this->validate_currency($to, $from)) {
                throw new Exception('Invalid currency code - must be exactly 3 letters');               
            }

            $response = $this->fetch($amount, $from, $to);

            if (isset($response['err'])) {
                throw new Exception('Invalid input');
            }

            $return = $response['v'];

            $this->new_cache($from.$to, $response['rate']);

        }

        return ($round) ? abs(round($return, 2)) : abs($return);

    }

    /*
    * Fetches data from external API
    */

    protected function fetch($amount, $from, $to)
    {
        $url    = "http://rate-exchange.appspot.com/currency?q={$amount}&from={$from}&to={$to}";
        $amount = (float) $amount;

        if (in_array('curl', get_loaded_extensions())) {

            $ch = curl_init();
            curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
            curl_setopt($ch, CURLOPT_URL, "http://rate-exchange.appspot.com/currency?q={$amount}&from={$from}&to={$to}");

            $response = json_decode(curl_exec($ch), true);
        }
        else {
            $response = json_decode(file_get_contents($url), true);
        }

        # Caches the rate for future
        $this->new_cache($from.$to, $response['rate']);

        return $response;       

    }

    /*
    * Checks if file is cached then returns rate
    */

    protected function get_cache($file) {

        if ($this->cachable && file_exists($this->cacheFolder.strtoupper($file).'.convertcache')) {             
            $file = file($this->cacheFolder.$file.'.convertcache');

            if ($file[0] < (time() - $this->cacheTimeout)) {    
                return FALSE;               
            }

            return $file[1];                        
        }

        return FALSE;

    }

    /*
    * Calculates amount needed in currency to achieve finish currency
    *
    * Set $round to FALSE to get full value
    */

    public function amount_to($finalAmount, $from, $to, $round = TRUE)
    {
        $finalAmount = (float) $finalAmount;

        if ($finalAmount == 0) {            
            return 0;
        }

        if (!$this->validate_currency($from, $to)) {        
            throw new Exception('Invalid currency code - must be exactly 3 letters');           
        }

        # Gets the rate
        $rate = $this->get_rate($from, $to);

        # Work it out
        $out = $finalAmount / $rate;

        return ($round) ? round($out, 2) : $out;
    }

    /*
    * Returns rate of two currencies
    */

    public function get_rate($from = 'GBP', $to = 'USD')
    {

        # Check cache

        $rate = $this->get_cache($from.$to);

        if (!$rate) {           
            $rate = $this->fetch(1, $from, $to);
            $rate = $rate['rate'];          
        }

        return $rate;

    }

    /*
    * Deletes all .convertcache files in cache folder
    */

    public function clear_cache()
    {
        $files = glob($this->cacheFolder.'*.convertcache');

        if (!empty($files)) {
            array_map('unlink', $files);           
        }

    }

    /*
    * Validates the currency identifier
    */

    protected function validate_currency()
    {
        foreach (func_get_args() as $val) {     
            if (strlen($val) !== 3 || !ctype_alpha($val)) {         
                return FALSE;               
            }
        }

        return TRUE;    

    }

    /*
    * Checks if file is cacheable then creates new file
    */

    protected function new_cache($file, $rate)
    {

        if ($this->cachable) {          
            $file = strtoupper($file).'.convertcache';

            $data = time().PHP_EOL.$rate;
            file_put_contents($this->cacheFolder.$file, $data);         
        }

    }

}

Usage:

<?php

require_once 'classes/convert.php';

$convert = new Convert;

# Convert 15 USD to GBP
echo $convert->convert(10, 'USD', 'GBP');

echo '<br/>';

# Displays how much USD you need to get 100 INR - won't show the rounded value
echo $convert->amount_to(100, 'USD', 'INR', FALSE);

Just made this class. Supports caching. I'd really appreciate any comments on how to make it better etc, just looking to improve!

Thanks :)

share|improve this question
add comment

1 Answer

Your class does too much. Split it up into multiple pieces:

  1. It converts currencies.
  2. It validates currency identifier.
  3. It fetches HTTP resources
  4. It caches fetched HTTP resources.

Although it does plenty of things, it has no answer to the problem of how to actually recognize which currency any amount is in. Adding two amounts might be valid, because you add GBP and GBP, but might actually be invalid because of GBP and USD. Your variable would only contain the integer or float value.

So split it up. First create a class that actually represents an amount of money in a specified currency. This can be as easy as making a class with two public values, $amount and $currency, but usually you do not want to allow write access to these, so the two values should go into the constructor, stored as private properties, and be accessible via get methods.

class Money_Currency
{
    /**
     * @var float
     */
    private $_amount;

    /**
     * @var string
     */
    private $_currency;

    public function __construct($amount, $currency)
    {
        $this->_amount = $amount;
        $this->_currency = $currency;
    }

    public function getAmount()
    {
        return $this->_amount;
    }

    public function getCurrency()
    {
        return $this->_currency;
    }
}

Adding or subtracting monetary values is a common task. How can we add two amounts of the same currency? Simple addition. Let's add a method for it. Note that I add the method to the Money_Currency class, which can be discussed. If I do not want to do this, I'd need an independent class that does all the math. If you have such a class, try this different approach. If not, continue following me...

    public function addAmount(Money_Currency $money)
    {
        if ($this->_currency !== $money->getCurrency()) {
            throw new InvalidArgumentException('Can only add money from the same currency');
        }

        $this->_amount += $money->getAmount();
    }

So now we are able to do a simple math operation:

$m1 = new Money_Currency(20, 'USD');
$m2 = new Money_Currency(30, 'USD');

$m1->addAmount($m2);

echo $m1->getAmount() . " " . $m1->getCurrency();

Outputs

50 USD

Easy. And completely unrelated to your currency conversion so far, but it solves a problem you might have, unless you are only offering a web service that inputs amount and currencies and translates this to the other value.

What about currency conversion? What about adding two different currencies? Decorator pattern to the rescue!

You can build a decorator that implements the same interface, which wraps around a currency object and does the calculations for converting the currency.

Let's fix the interface stuff first:

interface Money_Currency
{
    public function getAmount();

    public function getCurrency();
}

class Money_Currency_Value implements Money_Currency
{
    // instead of Money_Currency class from above

Then the converter:

class Money_Currency_Converter implements Money_Currency
{
    /**
     * @var Money_Currency
     */
    private $_money;

    /**
     * @var float
     */
    private $_conversionrate;

    /**
     * @var string
     */
    private $_sourcecurrency;

    /**
     * @var string
     */
    private $_targetcurrency;

    public function __construct($conversionrate, $sourcecurrency, $targetcurrency)
    {
        $this->_conversionrate = $conversionrate;
        $this->_sourcecurrency = $sourcecurrency;
        $this->_targetcurrency = $targetcurrency;
    }

    public function setMoney(Money_Currency $money)
    {
        if ($this->_sourcecurrency !== $money->getCurrency()) {
            throw new InvalidArgumentException('The money value is in an incorrect currency for this converter');
        }

        $this->_money = $money;
    }

    public function getAmount()
    {
        return $this->_money->getAmount() * $this->_conversionrate;
    }

    public function getCurrency()
    {
        return $this->_targetcurrency;
    }
}

Now some test (continues with the objects from above):

$m3 = new Money_Currency_Value(10, 'GBP');

$convertGbpToUsd = new Money_Currency_Converter(1.5, 'GBP', 'USD');

$convertGbpToUsd->setMoney($m3);

echo "Converted ". $m3->getAmount() ." ". $m3->getCurrency() . " into ". $convertGbpToUsd->getAmount() . " " . $convertGbpToUsd->getCurrency();

try {
    $m1->addAmount($m3); // fails
} catch (Exception $e) {
}

$m1->addAmount($convertGbpToUsd);

echo $m1->getAmount() . " " . $m1->getCurrency();

Output:

Converted 10 GBP into 15 USD
65 USD

(we started with the 50 USD object from above, the conversion rate is completely arbitrary).

What have we got now? We can add amounts of the same currency. We can convert an amount into a different currency. We can also add amounts in different currencies via wrapping them into a converter first. That's pretty much all currency stuff should do.

Now how do you get this nice converter? It is simply a call to a class that generates it for you. This class should only deal with generating the proper conversion object when asked to provide one for conversion from currency A to B. To fulfill this, it needs access to a resource of knowledge, but this resource could be anything. For example, a HTTP client. But the client cannot be universal, it has to be customized for the web service you are using. So in reality you need to create something like a factory that is able to make a request for converting currencies to a certain web service. This call triggers a basic HTTP request with some parameters.

Caching should be done as decorator pattern as well. Decorating an HTTP call with a cache means that in the decorator you see if there is a matching entry in the cache that is still valid (might expire to fetch new updates), if not, it forwards the function call to the real client.

I apologize for not going into details for these tasks at this moment. I hope you got the idea of how to split up responsibilities between classes from the currency conversion example.

share|improve this answer
    
Some more thoughts on your current implementation: What happens if the curl extension is not present? What if I want to use a different web service? What if my conversion rates are inside a database? What if I want to use a different cache system like memcache? How about unit testing the whole stuff? But also: How is the programmer supposed to use this stuff in an easy way. Your approach is completely easy, but not configurable, adaptable and extendable for different uses. –  Sven Dec 5 '12 at 22:23
    
Thank you so much for that! So, by splitting it up, should I have the separate classes in different files, or the multiple classes in the same file? When people use this, I don't want there to be too many files where it can get confusing with directories etc –  Prash Dec 7 '12 at 17:17
    
Autoloading will solve this. And if you offer your module via Composer, then it will take care of this. However, the standard is to put one class per file, with a reasonable naming scheme. Read the "PSR-0" standard on how to correctly name your classes: github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md –  Sven Dec 7 '12 at 17:41
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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