I'm creating a website that parses in currency rates of the three major cities listed below in the $cities array.


// Feed URL's //
$theMoneyConverter = 'http://themoneyconverter.com/rss-feed/';

// Define arrays // 
$cities = array(
    'New York',

$currencyCode = array(

// Arguments parsed onto theMoneyConverter URL to set position of the <item> tags declared in the XML document
$currencySource = array(
    $theMoneyConverter . $currencyCode[0] . '/rss.xml?x=15',
    $theMoneyConverter . $currencyCode[1] . '/rss.xml?x=16',
    $theMoneyConverter . $currencyCode[2] . '/rss.xml?x=56'

Above is my configuration script that a developer will use if they wish to drop a city or add a new one. Notice in my $currencySource array that I add arguments to the end of each URL. For example, at index 0 in the array I add ?x=15. This argument corresponds to the <item> element from the original source feed from themoneyconvert.com. In the case of ?x=15, this would be the 15th <item> element from the XML document.

The XML document to confirm this. The 15th element contains the following element just to clarify:


The following is stored in a separate script called getCurrency.php:

function get_currency_rate($currencySource) {

      try {
        $xml = new SimpleXmlElement(file_get_contents($currencySource));
    } catch (Exception $e) {
        echo 'Caught exception: An error has occured. Unable to get file contents';

    $vars = parse_url($currencySource, PHP_URL_QUERY);
    switch ($x) {
        case 15:
            get_rate($xml, 15); //EUR 15
            get_rate($xml, 56); //USD 56
        case 16:
            get_rate($xml, 16); //GBP 16
            get_rate($xml, 15); //EUR 15
        case 56: default :
            get_rate($xml, 15); // EUR 15
            get_rate($xml, 56); // USD 56

// Get and return currency rate
function get_rate(SimpleXMLElement $xml, $x) {

    $currency['rate'] = $xml->channel->item[$x]->description;

    echo $currency['rate'], '<br />';

I call the get_currency_rate() function from my user interface like such:

echo get_currency_rate($currencySource[$index])

After getting the file contents I extract the argument that I added to the URL from the configuration file listed above and store it as a variable $x. The XML feed and variable $x are parsed into the get_rate function().

The whole process doesn't seem very maintainable and particular readable. Adding a new city into the equation would become a nightmare for another developer. Can anyone think of any other alternatives?

  • \$\begingroup\$ I just ran your url with ?x=15 and 16 - it does not grab the 15th element. it returns the entire XML. \$\endgroup\$ – azngunit81 Apr 10 '14 at 4:05

I see some inconsistencies in your getCurrency.php code. There are many variables that are never declared within the functions, so I don't think that's going to work as expected.

I think the best approach would be to make a parser which looks for whatever currency code you need, regardless of the item order because that might change in the future. I mean, the most performance-expensive part of the job is to parse the whole XML into a SimpleXML structure which you're doing anyway, so looping through the items seaching for the right one wouldn't be much worse and your code will be more straight-forward and escalable.

On the other hand, you might also want to have some cache of the parsed XML to avoid re-parsing the same file on each request. But that depends on the variability of the XML feed. Maybe you can check with The Money Converter how often the feed is updated.

| improve this answer | |

At configuration script you really need two (three) indexed arrays? I prefer associative arrays, or Objects.

At getCurrency.php You catch the exception, but the code doesn't exit, and You are using Exception to handle two types of error, the file_get_contents returned false or the returned XML was invalid. At the first case you create an Exception which is an expensive opertaion.

parse_url + parse_str is not the cleanest way to read the parameter and confusing. Instead of using indexed arrays and get paramters You should create an Object, and You don't need to trick with the url.

The switch is horrible, the only thing what I can say is throw it out.

Missing error handler get_rate if there is no record.

My suggestion is to learn Object Oriented Paradigm, make Your life better, and Your code maintainable.

| improve this answer | |

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.