Page 1 of 1

Could someone please check to see if this is valid code?

Posted: Fri Sep 30, 2016 12:34 am
by planetalk
I want to direct customers to different order pages depending on the country that they are ordering from so I can offer their own currency. This is what I'm using but I can't really check to see if it works without hopping on an airplane. I have tried a couple of proxy type sites but they don't seem very reliable.

Is the code below valid? Thanks a lot.

Code: Select all

<?php
$a = unserialize(file_get_contents('http://www.geoplugin.net/php.gp?ip='.$_SERVER['REMOTE_ADDR']));
$countrycode= $a['geoplugin_countryCode'];
if ($countrycode=='AU' OR $countrycode=='NZ')
    header( 'Location: http://www.planetalkguitar.com/orderau.html' ) ;
else if ($countrycode=='GB')
    header( 'Location: http://www.planetalkguitar.com/orderuk.html' ) ;
else 
    header( 'Location: http://www.planetalkguitar.com/orderus.html' ) ;

?>

Re: Could someone please check to see if this is valid code?

Posted: Fri Sep 30, 2016 2:04 am
by requinix
You shouldn't unserialize data from an unknown source, even if it is trustworthy. There's also no guarantee that serialized data is the same across versions of PHP.

It's an easy enough change to switch to JSON.

Code: Select all

<?php
$a = json_decode(file_get_contents('http://www.geoplugin.net/json.gp?ip='.$_SERVER['REMOTE_ADDR']));
$countrycode= $a['geoplugin_countryCode'];
if ($countrycode=='AU' OR $countrycode=='NZ')
    header( 'Location: http://www.planetalkguitar.com/orderau.html' ) ;
else if ($countrycode=='GB')
    header( 'Location: http://www.planetalkguitar.com/orderuk.html' ) ;
else 
    header( 'Location: http://www.planetalkguitar.com/orderus.html' ) ;

?>
Yes, the code is valid. But here's another piece of advice: to avoid hitting that API every time someone happens to land on... whatever page triggers it, you should set a cookie to remember where they are. Which is also an easy change.

Code: Select all

<?php
$a = json_decode(file_get_contents('http://www.geoplugin.net/json.gp?ip='.$_SERVER['REMOTE_ADDR']));
$countrycode= $a['geoplugin_countryCode'];
setcookie('countryCode', $countrycode, time() + 86400, '/', '.planetalkguitar.com');
if ($countrycode=='AU' OR $countrycode=='NZ')
    header( 'Location: http://www.planetalkguitar.com/orderau.html' ) ;
else if ($countrycode=='GB')
    header( 'Location: http://www.planetalkguitar.com/orderuk.html' ) ;
else 
    header( 'Location: http://www.planetalkguitar.com/orderus.html' ) ;

?>

Re: Could someone please check to see if this is valid code?

Posted: Fri Sep 30, 2016 5:36 am
by planetalk
Thanks so much, requinix, I appreciate it. I don't even know what JSON is, never heard of it, but I will make the changes you suggest.

If I wanted to add a new batch of countries (like those that use the Euro), do I just carry on with the same lines of code starting with 'else if'?

Oops ... I just changed the code to your suggestion and this came up:

Fatal error: Cannot use object of type stdClass as array in /home/ptguitar/public_html/order.php on line 3

I switched back to the old code. Any idea why that happened?

Re: Could someone please check to see if this is valid code?

Posted: Fri Sep 30, 2016 5:53 am
by requinix
planetalk wrote:If I wanted to add a new batch of countries (like those that use the Euro), do I just carry on with the same lines of code starting with 'else if'?
You could. An alternative to a lot of if/else statements is a switch:

Code: Select all

switch ($countrycode) {
	case 'AU':
	case 'NZ':
		header('Location: http://www.planetalkguitar.com/orderau.html');
		break;
	case 'GB':
		header('Location: http://www.planetalkguitar.com/orderuk.html');
		break;
	default:
		header('Location: http://www.planetalkguitar.com/orderus.html');
		break;
}
You could also rewrite the code to make it easier to deal with - less copy and paste.

Code: Select all

$pages = array(
	'AU' => 'orderau.html',
	'NZ' => 'orderau.html',
	'GB' => 'orderuk.html',
	0 => 'orderus.html' // default
);
$page = (isset($pages[$countrycode]) ? $pages[$countrycode] : $pages[0]);
header('Location: http://www.planetalkguitar.com/' . $page);
This way you only have to update the $pages array.

Even easier would be to make the files be orderau.html, ordernz.html, and ordergb.html (and orderus.html) according to the country code and do

Code: Select all

$page = 'order' . strtolower($countrycode) . '.html';
if (is_file($page)) {
	header('Location: http://www.planetalkguitar.com/' . $page);
} else {
	header('Location: http://www.planetalkguitar.com/orderus.html');
}
which redirects to a order*.html file if present or the US one if not.
planetalk wrote:Oops ... I just changed the code to your suggestion and this came up:

Fatal error: Cannot use object of type stdClass as array in /home/ptguitar/public_html/order.php on line 3

I switched back to the old code. Any idea why that happened?
My fault. Should be

Code: Select all

$a = json_decode(file_get_contents('http://www.geoplugin.net/json.gp?ip='.$_SERVER['REMOTE_ADDR']), true);
By default json_decode turns stuff into objects, which means the next line would need to have "$a->geoplugin_countryCode". You could do that, or you could pass that "true" to make it turn stuff into arrays instead.

Re: Could someone please check to see if this is valid code?

Posted: Fri Sep 30, 2016 6:28 am
by planetalk
Cool. Bedtime here in Oz, I'll try it all out tomorrow. Thanks, mate.