Code Tidying Help

[LemonInflux]

<?php

/* The array for the function looks like this:

$array = array
(
'title' => 'title of page',
'content' => 'page content'
);

*/

function pageCreate($array)
{
/* This is the style the user has opted for. All this does is give the function
a path to follow to find the file it needs to include */
global $style;

/* This gets the template file, ready for editing for create the page */
$page = file_get_contents('includes/page_templates/'. $style .'/page.html');

/* Here, we check that the title and content keys exist (the neccesary element */
if(!array_key_exists('title', $array) || !array_key_exists('content', $array))
{
	die('There was an error with the template construction. Title or Content array key undefined.');
}

/* Now, we give them variables. */
$title = $array['title'];
$content = $array['content'];

/* On the template, there will be an area for login status. This is where we alter the template text ('{LOGIN}') to the
user's login status. */
if(isset($_SESSION['user']))
{
	$page = str_replace('{LOGIN}', 'Logged in as '. $_SESSION['user'] .' [ <a href="?logout=true">Log Out</a> ]', $page);
}
else
{
	$page = str_replace('{LOGIN}', 'You are not logged in.', $page);
}

/* This is the bit I'd like to know if there's any way I could tidy it. What this does is as follows:
inside the original array, you can also put 6 other values. (we'll look at the last 3 later, the first three
are the important ones at the moment). These are t2, t3, and t1. On the template, there is space for
3 tabs (buttons to other pages). To make one display as currently selected, and the others to display
as inactive, I put a * inside the name of the active tab. What this bit does is finds the tab that is meant
to be active, and makes it active. */
if(isset($array['t2']) && strpos($array['t2'], '*'))
{
	$C2 = 'active';
	$T2 = str_replace('*', '', $array['t2']);
}
elseif(isset($array['t3']) && strpos($array['t3'], '*'))
{
	$C3 = 'active';
	$T3 = str_replace('*', '', $array['t3']);
}
else
{
	$C1 = 'active';
	$T1 = str_replace('*', '', $array['t1']);
}

/* This works out which tabs we need to use, and makes the ones that aren't active
inactive. This is also very messy.

If we have a value for $array['tX'], then we should also have a value for $array['tXl']
(where you want tX to link to). If this isn't set, the tab will be unclickable. */
if(isset($array['t2']) && !isset($T2))
{
	$C2 = 'inactive';

	if(array_key_exists('t2l', $array))
	{
		$T2 = '<a href="'. $array['t2l'] .'">'. $array['t2'] .'</a>';
	}
	else
	{
		$T2 = $array['t2'];
	}
}
elseif(!isset($array['t2']))
{
	$C2 = 'panelblank';
	$T2 = '';
}

if(isset($array['t3']) && !isset($T3))
{
	$C3 = 'inactive';

	if(array_key_exists('t3l', $array))
	{
		$T3 = '<a href="'. $array['t3l'] .'">'. $array['t3'] .'</a>';
	}
	else
	{
		$T3 = $array['t3'];
	}
}
elseif(!isset($array['t3']))
{
	$C3 = 'panelblank';
	$T3 = '';
}

if(isset($array['t1']) && !isset($T1))
{
	$C1 = 'inactive';

	if(array_key_exists('t1l', $array))
	{
		$T1 = '<a href="'. $array['t1l'] .'">'. $array['t1'] .'</a>';
	}
	else
	{
		$T1 = $array['t1'];
	}
}
elseif(!isset($array['t1']))
{
	$C1 = 'panelblank';
	$T1 = '';
}

/* This creates the final array to replace fields in the template
with the relevant information for the page template. */
$replace = array(
	// The title
	'{TITLE}' => $title,

	// The 3 tab classes (active, inactive, or panelblank)
	'{C1}' => $C1,
	'{C2}' => $C2,
	'{C3}' => $C3,

	// The 3 tab names
	'{T1}' => $T1,
	'{T2}' => $T2,
	'{T3}' => $T3,

	// The actual content
	'{CONTENT}' => $content
);

// And here, we build the page.
$output = str_replace(array_keys($replace), array_values($replace), $page);

// Finally, the function outputs the page.
echo $output;
}

?>

 

Quite a long code, I know. But looking back on it, it's really quite messy compared to the rest of my code (I've been tidying it up to make it a little more organized). I've looked through, and I can't find any obvious ways of tidying this up (maybe there are some ways, and I'm just missing them). I'd be greatful for any help. Thanks in advance,

 

Tom

[PHP Monkeh]

It depends on your coding style, people use many different ways of using braces.  Personally I prefer putting them on the same line as my function, but I can see you're using the style of putting them on their own line.

 

It's just down to preference, where you use:

 

if(!array_key_exists('title', $array) || !array_key_exists('content', $array))
{

 

I would use:

 

if(!array_key_exists('title', $array) || !array_key_exists('content', $array)) {

 

Then I would see that as being neater.  But it's all down to personal opinion.  If you feel like you can read the code and understand its flow, then there isn't much you can do to make it "neater".

[LemonInflux]

Yeah, it's just like, where I've put if($var = 1){ this }elseif($var = 2){ this }else{ this}, I was wondering if there was a way to...condense it a bit more.