Which is more efficient? Ideas appreciated!

[Bottyz]

Hi all,

 

 

I've been trying to improve the speed of my file download script and was wondering if anyone could advise me which of the following is more efficient (Don't worry its not the whole script, just one segment), in terms of speed and server load?

 

The way I have the segment currently:

 

//if file exists need to check authorision levels	
//set access to no
$access = NULL;
//retrieve current user levels
$cpm = $_SESSION['MM_CPMGroup'];
$cpmh = $_SESSION['MM_CPMHGroup'];
$cm = $_SESSION['MM_CMGroup'];
$cj200 = $_SESSION['MM_CJ200Group'];
$cj = $_SESSION['MM_CJGroup'];
//set file category type & set access if allowed
if ($category == 'cpm') {
	if ($cpm == '1') {
		$access = 1;
		if ($subcategory == 'techdata') {
			$path = "files/techdata/cpm/";
		}
		elseif ($subcategory == 'msds') {
			$path = "files/techdata/cpm/msds/";
		}
		elseif ($subcategory == 'symbols') {
			$path = "files/symbols/cpm/";
		}
		else {
		$path = "files/cpm/";
		}
	}
}
elseif ($category == 'cpmh') {
	if ($cpmh == '1') {
		$access = 1;
		if ($subcategory == 'techdata') {
			$path = "files/techdata/cpmh/";
		}
		elseif ($subcategory == 'msds') {
			$path = "files/techdata/cpmh/msds/";
		}
		elseif ($subcategory == 'symbols') {
			$path = "files/symbols/cpmh/";
		}
		else {
		$path = "files/cpmh/";
		}
	}
}
elseif ($category == 'cm') {
	if ($cm == '1') {
		$access = 1;
		if ($subcategory == 'techdata') {
			$path = "files/techdata/cm/";
		}
		elseif ($subcategory == 'msds') {
			$path = "files/techdata/cm/msds/";
		}
		elseif ($subcategory == 'symbols') {
			$path = "files/symbols/cm/";
		}
		else {
		$path = "files/cm/";
		}
	}
}
elseif ($category == 'cj200') {
	if ($cj200 == '1') {
		$access = 1;
		if ($subcategory == 'techdata') {
			$path = "files/techdata/cj200/";
		}
		elseif ($subcategory == 'msds') {
			$path = "files/techdata/cj200/msds/";
		}
		elseif ($subcategory == 'symbols') {
			$path = "files/symbols/cj200/";
		}
		else {
		$path = "files/cj200/";
		}
	}
}
elseif ($category == 'cj') {
	if ($cj == '1') {
		$access = 1;
		if ($subcategory == 'techdata') {
			$path = "files/techdata/cj/";
		}
		elseif ($subcategory == 'msds') {
			$path = "files/techdata/cj/msds/";
		}
		elseif ($subcategory == 'symbols') {
			$path = "files/symbols/cj/";
		}
		else {
		$path = "files/cj/";
		}
	}
}
if ($access < 1) {
	// if user access not granted to file category return message
	if($logging > 0){
		$status = "Wrong Permissions";
		include('logit.php');
	}
	if (! $_SESSION['PrevUrl']) {
		//header("Location: ". $loginpage );
		exit;
	}
	$redirect = $_SESSION['PrevUrl'];
	header("Location: ". $redirect );
	exit;
}
// if file exists and user access granted continue

 

Obviously the above is a lot of lines of code... So I have rewritten the above to look like:

 

//if file exists need to check authorision levels & retrieve current user levels

if ($category == 'cpm' && $_SESSION['MM_CPMGroup'] == '1') {
	$access = 1;
} elseif ($category == 'cpmh' && $cpmh = $_SESSION['MM_CPMHGroup'] == '1') {
	$access = 1;
} elseif ($category == 'cm' && $cm = $_SESSION['MM_CMGroup'] == '1') {
	$access = 1;
} elseif ($category == 'cj200' && $_SESSION['MM_CJ200Group'] == '1') {
	$access = 1;
} elseif ($category == 'cj' && $_SESSION['MM_CJGroup'] == '1') {
	$access = 1;
} else {
	$access = NULL;
}

if ($access == NULL) {
	// if user access not granted to file category return message
	$status = "Unauthorised";
	include('logit.php');
	header("Location: ".$_SESSION['PrevUrl']);
	exit;
}
// if file exists and user access granted continue

switch($subcategory) {
	case "techdata":$path="files/techdata/".$category."/".$filename; break;
	case "msds":	$path="files/techdata/".$category."/msds/".$filename; break;
	case "symbols":	$path="files/symbols/".$category."/".$filename; break;
	default:		$path="files/".$category."/".$filename;
}

 

The second version is a lot shorter, but is it better? And could I shorten the if statement further so its more like:

 

//if file exists need to check authorision levels & retrieve current user levels

if (($category == 'cpm' && $_SESSION['MM_CPMGroup'] == '1') || ($category == 'cpmh' && $cpmh = $_SESSION['MM_CPMHGroup'] == '1') || ($category == 'cm' && $cm = $_SESSION['MM_CMGroup'] == '1') || ($category == 'cj200' && $_SESSION['MM_CJ200Group'] == '1') || ($category == 'cj' && $_SESSION['MM_CJGroup'] == '1') {
	$access = 1;
} else {
	$access = NULL;
}

if ($access == NULL) {
	// if user access not granted to file category return message
	$status = "Unauthorised";
	include('logit.php');
	header("Location: ".$_SESSION['PrevUrl']);
	exit;
}
// if file exists and user access granted continue

switch($subcategory) {
	case "techdata":$path="files/techdata/".$category."/".$filename; break;
	case "msds":	$path="files/techdata/".$category."/msds/".$filename; break;
	case "symbols":	$path="files/symbols/".$category."/".$filename; break;
	default:		$path="files/".$category."/".$filename;
}

 

Any advice would be appreciated! Thanks!!

 

 

 

[colleyboy]

What holds things up a lot is when there are more calls to the php parser so in my opinion if you can write shorter code with as little calls to the parser the page is going to load quicker.

 

If you do that you must check your security too though.

[ignace]

The 3th script is better. Less code that can do the same job is better, everything else is just noise.

 

What holds things up a lot is when there are more calls to the php parser so in my opinion if you can write shorter code with as little calls to the parser the page is going to load quicker.

 

Your script does not call the parser instead on each request it's invoked and "parses" your script to machine-readable code (so called op-code) and executes it.