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

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
planetalk
Forum Newbie
Posts: 3
Joined: Fri Sep 30, 2016 12:29 am

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

Post 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' ) ;

?>
User avatar
requinix
Spammer :|
Posts: 6617
Joined: Wed Oct 15, 2008 2:35 am
Location: WA, USA

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

Post 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' ) ;

?>
planetalk
Forum Newbie
Posts: 3
Joined: Fri Sep 30, 2016 12:29 am

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

Post 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?
User avatar
requinix
Spammer :|
Posts: 6617
Joined: Wed Oct 15, 2008 2:35 am
Location: WA, USA

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

Post 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.
planetalk
Forum Newbie
Posts: 3
Joined: Fri Sep 30, 2016 12:29 am

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

Post by planetalk »

Cool. Bedtime here in Oz, I'll try it all out tomorrow. Thanks, mate.
Post Reply