Style of my php script

Hi,

I have a question about how I’ve built a small AJAX application. Mostly PHP related though.

I have an ajax call that calls to the server to a PHP script which builds a html snippet containing feed data from Twitter, Facebook,Rss and custom Xml.

The script calls to the server, sending some params which are then checked for at the top of the php and assigned to variables. From here a function is called:


Check for the request from the jquery call and assign to variables
if(isset($_GET['twitterQuery'])) {
    $twitterQuery = $_GET['twitterQuery'];    
}

if(isset($_GET['rssQuery'])) {
    $rssQuery = $_GET['rssQuery'];    
}

if(isset($_GET['facebookQuery'])) {
    $facebookQuery = $_GET['facebookQuery'];    
}


 buildFeed($twitterQuery,$rssQuery,$facebookQuery);

 

That’s it. No class or anything. I don’t really know if a class would the right thing to use here.

The buildfeed function is an init function and it in turn calls another 2 functions in the script. I’m grabbing Json from facebook and xml from the rest so I needed two separate functions really. Both do some checking with cached data and if it’s time to check again then a call is made, returned data is formatted and the whole lot is ordered and it’s all sent back in a <ul>.

My questions are:

  • does this sound like the right way to approach this? I don’t like that it’s all in the global scope, it’s totally procedural, it hinges somewhat on the ajax call being made successfully.
    -Is it correct form to have the init that then calls other functions.
    -How else can I initialise the php script from the ajax call? Currently the ajax just calls the server, sending some params, then there’s a bunch of check to assign the params ta vars on the php side, which then calls a init. Is that what you’d do?
    -Should this being in a class?
  • I collect all the get date in if blocks and else blocks at the top. If it’s set the use that value if not then use a default value. Is that a safe strategy do you think?
  • I could really do with some global vars that were available to all functions. But how to approach this in php

Sorry a few small questions there :wink: I just need some ideas on how to fine tune it a bit. I’d appreciate any thoughts.

I dislike it just because everything is hard-coded… every time you add a new one or remove a valid one you’re recoding large sections of code.

$feedTypes=array(
	'twitterQuery',
	'rssQuery',
	'facebookQuery'
)

$feedList=array();

foreach ($feedTypes as $feed) {
	if (isset($_GET[$feed])) {
		$feedList[]=array(
			'fieldType' => $feed,
			'fieldRequest' => $_GET[$feed]
		);
	}
}

buildFeed($feedList);

Would let you add/remove them easily – just change buildfeed to work off the array.

Though at that point, since each of those feeds likely already has a source/type, I’d probably pass:

?queryFeeds=comma,delimited,list;

and then set the storage type in the array.


$feedTypes=array(
	'twitter' => 'json',
	'rss' => 'atom',
	'facebook' => 'xml'
)

Wait… or are you passing the URL as the data? Not really clear on that given we’re not seeing what’s requesting the info. Just what is being passed for data in the various $_GET, the type or the URL?

If that is the URL being passed, I’d probably be doing something like this:


$feedTypes=array(
	'twitter' => 'json',
	'rss' => 'atom',
	'facebook' => 'xml'
);

$feedList=array();

foreach ($feedTypes as $feed => $feedFormat) {
	if (isset($_GET[$feed.'Query'])) {
		$feedList[]=array(
			'fieldSource' => $feed,
			'fieldType' => $feedFormat,
			'fieldRequest' => $_GET[$feed.'Query']
		);
	}
}

if (count($feedList)>0) buildFeed($feedList) else die('No valid request!');

Then if I’m worried about global scope, that’s what UNSET is for.

As to making a class out of it – how often is it called for each server request? How many different instances are running at the same time in the same script? IF the answer for those is once per, there is NO REASON to be throwing the overhead of objects at it.

Procedural isn’t “evil” – and objects are NOT the answer to every problem.

Hi DeathShadow,

Thanks for taking the time to make an excellent reply which has cleared a lot up for me.

I like that approach. So only feeds that are declared in the feedtypes array are only ever called.

Thanks for clearing up the class stuff also :slight_smile: