PHP isset over use? Good or bad?

Question

First, I'm in no way an experienced PHP coder. This is my 5th time working with PHP, so if you see anything that can bee improve, please point them out for me.

I have the code checking for the $_GET variable and use them to create the page title:

<?php
$pgeName = safeVal(basename($_SERVER['PHP_SELF'], '.php'));
$pgeBreadcumbs .= '<li><i class="fa fa-chevron-right"></i><a href="'.$siteurl.'" class="m5b">Front Page</a></li>';
$pgeBreadcumbs .= '<li><i class="fa fa-chevron-right"></i><a class="m5b">Free website</a></li>';
if ($pgeName == 'index') {
 $query = 'SELECT title, description, keywords FROM rff_pageTitle WHERE pge = \'default\' LIMIT 1';
 $pge = mysqli_fetch_assoc(rff_query($query));
 $pgeIfo = $pge;
 $pgeIfo['breadcrumbs'] = $pgeBreadcumbs;
} else {
 if (isset($_GET['sortby'])) {
 $sortby = safeVal($_GET['sortby']);
 if ($pgeName == 'browsegallery') {
 switch ($sortby) {
 case 'mostpopular' : $sortby = 'Most Popular'; break;
 case 'heighestrated' : $sortby = 'Highest Rated'; break;
 case 'mostviewed' : $sortby = 'Most Viewed'; break;
 default : $sortby = $sortby;
 }
 }
 }
 $pge_num = 1;
 if (isset($_GET['page']))
 $pge_num = safeVal($_GET['page']);
 if (isset($_GET['slug'])) {
 $slug = safeVal($_GET['slug']);
 $anm_nme = rff_getAnimeName($slug);
 $anm_id = getIDbySlug($slug);
 $anm_tpe = (rff_getAnimeType($anm_id) == 1) ? 'JAV' : 'H-Anime';
 $anm_pdc = '-';
 }
 if (isset($_GET['s']))
 $s = safeVal($_GET['s']);
 if (isset($_GET['tag']))
 $tag = safeVal($_GET['tag']);
 if (isset($_GET['startfrom']))
 $startfrom = safeVal($_GET['startfrom']);
 if (isset($_GET['param'])) {
 $param = safeVal($_GET['param']);
 $contriType = str_replace('-', ' ', $param);
 }
 if (isset($_GET['eps']))
 $eps = safeVal($_GET['eps']);
 if (isset($_GET['cond'])) {
 $cond = safeVal($_GET['cond']);
 $edt_cond = str_replace('-', ' ', $cond);
 }
 if (isset($_GET['id'])) {
 $id = safeVal($_GET['id']);
 if ($pgeName != 'zedit')
 $edt_id = getAnimeName($id);
 else {
 switch($cond) {
 case 'tag' : $temp = 'edt_tags'; break;
 case 'link' : $temp = 'new_eps'; break;
 case 'anime' : $temp = 'edt_anm'; break;
 case 'new-tag' : $temp = 'tags'; break;
 }
 $query = rff_query('SELECT nme FROM '.$temp.' WHERE id = '.$id.' LIMIT 1');
 $res = mysqli_fetch_assoc($query);
 $edt_id = $res['nme'];
 }
 }
 if (isset($_GET['subPge'])) {
 $subPge = safeVal($_GET['subPge']);
 switch ($subPge) {
 case 'fav' : $newPgeName = 'Favorites'; break;
 default : $newPgeName = $subPge;
 }
 } else {
 switch ($pgeName) {
 case 'mysettings' : $newPgeName = 'My Settings'; break;
 case 'myupload' : $newPgeName = 'My Uploads'; break;
 case 'mycomments' : $newPgeName = 'My Comments'; break;
 case 'myratings' : $newPgeName = 'My Ratings'; break;
 case 'mytags' : $newPgeName = 'My Tags'; break;
 default : $newPgeName = $pgeName;
 }
 }
 if (isset($_GET['chapterid'])) {
 $chapterid = safeVal($_GET['chapterid']);
 $query = rff_query('SELECT nme FROM new_eps WHERE id = '.$chapterid.' LIMIT 1');
 $res = mysqli_fetch_assoc($query);
 $eps_nme = $res['nme'];
 }
 if (isset($_GET['tageditid']))
 $tageditid = safeVal($_GET['tageditid']);
 if (isset($_GET['galeditid'])) {
 $galeditid = safeVal($_GET['galeditid']);
 $query = rff_query('SELECT nme FROM edt_anm WHERE id = '.$galeditid.' LIMIT 1');
 $res = mysqli_fetch_assoc($query);
 $ea_nme = $res['nme'];
 }
 $order = ($startfrom == '') ? 'A to Z' : $startfrom;
 $search = array(
 '@cond',
 '@pge_num',
 '@slug',
 '@s',
 '@tag',
 '@order',
 '@param',
 '@anm_nme',
 '@anm_pdc',
 '@anm_tpe',
 '@anm_eps',
 '@edt_cond',
 '@edt_id',
 '@pgeNme',
 '@eps_nme',
 '@ct_nme',
 '@te_nme',
 '@ea_nme',
 );
 $replace = array(
 ucwords($sortby),
 $pge_num,
 ucwords($slug),
 ucwords($s),
 ucwords($tag),
 ucwords($order),
 ucwords($contriType),
 $anm_nme,
 $anm_pdc,
 $anm_tpe,
 $eps,
 ucwords($edt_cond),
 $edt_id,
 ucwords($newPgeName),
 ucwords($eps_nme),
 rff_getTagName($slug, 'tags'),
 rff_getTagName($tageditid, 'edt_tags'),
 $ea_nme,
 );
 $query = 'SELECT title, description, keywords, breadcrumbs FROM rff_pageTitle WHERE pge = \''.$pgeName.'\' LIMIT 1';
 $data = rff_query($query);
 $num = mysqli_num_rows($data);
 $defaultTitle = 'My default title, if the page isnt in database';
 $defaultDesc = ' My default description, if the page isnt in database';
 $defaultKey = 'My default keyword, if the page isnt in database';
 if ($num > 0) {
 $pge = mysqli_fetch_assoc($data);
 $pgeIfo['title'] = str_replace($search, $replace, $pge['title']).$defaultTitle;
 $pgeIfo['description'] = str_replace($search, $replace, $pge['description']).$defaultDesc;
 $pgeIfo['keywords'] = ($pge['keywords'] == '') ? $defaultKey : $pge['keywords'];
 /** Breadcrumbs **/
 $breadcrumbs = str_replace($search, $replace, $pge['breadcrumbs']);
 $bcbArr = explode(",", $breadcrumbs);
 if ($bcbArr[0] != '')
 $pgeIfo['breadcrumbs'] .= '<li><i class="fa fa-chevron-right"></i><a href="" class="m5b">'.$bcbArr[0].'</a></li>';
 if ($bcbArr[1] != '')
 $pgeIfo['breadcrumbs'] .= '<li><i class="fa fa-chevron-right"></i><a href="" class="m5b">'.$bcbArr[1].'</a></li>';
 if ($bcbArr[2] != '')
 $pgeIfo['breadcrumbs'] .= '<li><i class="fa fa-chevron-right"></i><a href="" class="m5b">'.$bcbArr[2].'</a></li>';
 } else {
 $query = 'SELECT title, description, keywords FROM rff_pageTitle WHERE pge = \'default\' LIMIT 1';
 $pge = mysqli_fetch_assoc(rff_query($query));
 $pgeIfo = $pge;
 $pgeIfo['breadcrumbs'] = $pgeBreadcumbs;
 }
}
?>

Database Structure: id, slug, title, description, keywords, breadcumbs

Example Record:
(id, slug, title, description, keywords, breadcumbs)
(1, search, 'Search Results for @s@slug - Page @pge_num', 'Search results for @s@slug', '@s, @slug', 'Browsing, @s@slug, @pge_num')

The safeVal function use mysqli_escape_string function to escape the string.

As you can see, I have a bunch of if(isset(variable)) statements in the code.

My question are:

• Is this the best way to do it performance wise?
• Is there anyway to improve the code?
• Any other way to do this more efficiently?

• Josef Engelfrost: If you're having performance issues, isset() is not the problem. Focus on writing readable and maintainable code, if you (and others) find isset() readable, use isset().
• Josef Engelfrost: If you want to dig into performance start with Big O notation. You're current code is doing very well.
• Richard: @JosefOttosson : I'm not aware of any other way to catch $_POST variable other than islet, well i know that i can use extract($_POST) but ppl said that isn't a safe way to do it.
• Josef Engelfrost: All isset($var) does is check if $var is set and is not null. You are using it correctly. Do you want a review of the code as a whole, or are you mainly concerned about isset()?
• Josef Engelfrost: By the way, your code is missing an ending brace somewhere, and safeVal() and $pgeBreadcumbs are not set.

Answer 1

Isset should not affect performance.

About code improvements I suggest you to write some kind of wrapper above Request to improve readability

<?php 
class RequestData {
 public function __construct($val) {
 $this->val = $val;
 }
 public function raw() {
 return $this->val;
 }
 public function safe() {
 // just an example
 if (is_string($this->val))
 return htmlspecialchars($this->val);
 return $this->val;
 }
 public function int() {
 return int($this->val);
 }
 protected $val;
}
class Request {
 public function get($varName, $defaultValue = false) {
 if (isset($_GET[$varName]))
 return new RequestData($_GET[$varName]);
 return new RequestData($defaultValue);
 }
 public function post($varName, $defaultValue = false) {
 if (isset($_POST[$varName]))
 return RequestData($_POST[$varName]);
 return new RequestData($defaultValue);
 }
}
$_GET['test'] = '<script>alert();</script>';
$request = new Request();
$testVal = $request->get('test')->safe();
if ($testVal) {
 echo 'test exists: ' . $testVal;
} else {
 echo 'test does not exist';
}

---

Also connections like this:

case 'mysettings' : $newPgeName = 'My Settings'; break;

should be in config files or in DB. It shouldn't be directly in the code. Common way is to keep page data in DB:

• id
• title
• excerpt
• content
• slug
• createdAt
• category
• keywords
• description

If you have such table you are able to select page by slug or by id and then fill necessary information to display it to user.

If your page is not just a content page (e.g. search form) it's good to have separate controller to handle all request params and a separate view to render this form (including title and other stuff)

• Richard: yes i do have a database that stored all the information except for that part... That switch case is for the breadcumbs where I turn the slug into readable text hence the most viewed => Most Viewed. Added completed code! Thanks

Answer 2

This is just very minor, but when maintaining code, I'd rather have variables be longer, but the whole word, so I don't have to go back and try and recall the skewed spelling. But this is just a minor pet-peeve.