Making my code simpler....

darthmahon

Hi,

Ok starting my site again using CSS and I want to cut down on as much duplicate code as possible. Ok so here is the code to load one page of my site. I do this by putting a ?Section=Whatever in the url, it then picks out what Section is required and loads the relevant page.

<?
$Section = $_REQUEST['Section'];
$intSession =$_SESSION['level'];

$PremiumMembers = "scripts/members-premium.inc";
$FreeMembers = "scripts/members-only.inc";
// Determine What Main Page to include
If (isset($Section))
{

switch ($Section) {
    Case "UK":
        $PageLoad = "scripts/meetings-list.inc";
        $AccessLevel = 0;
    break;       
    Case "International":
        $PageLoad = "scripts/format2.inc";
        $AccessLevel = 2;
        $TableSelect = "international_meetings";
        $TableID = "14";
    break;
    Case "Calls":
        $PageLoad = "scripts/format2.inc";
        $AccessLevel = 2;
        $TableSelect = "calls";
        $TableID = "15";
    break;
}
}  Else
{
//Load index if section was not supplied
    $PageLoad = "scripts/meetings-index.inc";
    $AccessLevel = 0;
}

If ($intSession >= $AccessLevel)
{
    $PageLoad =$PageLoad;
}
Elseif ($AccessLevel == 3)
{
    $PageLoad =$PremiumMembers;
}
ElseIf ($AccessLevel == 2)
{
    $PageLoad =$FreeMembers;
}
include ($PageLoad);
?>

What I want to know is a lot of my pages have a similar format, they have text that comes from the database and a list of other information in a specific table. So, above the sections International and Calls have the same format. They have text and select from the same table.

So in the file format2.inc I want something like this:

<?

$query = "SELECT * FROM $TableSelect WHERE id = $TableID";

$result = mysql_query($query) or die ("Error in query: $query. " . mysql_error());

// Ouput Code

?>

Is what I am doing correct and will it work?

Many thanks,
Chris

Gary_King

Have you tested it first to see if it'll work?

dartcol

I imagine it comes down to how fast you want the queries to be. I read somewhere that using SELECT * is slower than naming the column names. I can't see why your code might not work. Like Gary W says try it and see. Also, make a note of the speed of the query.

laserlight

I'm not too clear on the flow of logic on the first snippet, bue generally you have a redundant statement:
$PageLoad =$PageLoad;
that can be removed.
Also, you might want to consider using the keywords (e.g. if) as all lowercase, as that seems more standard.

$Section = $REQUEST['Section'];
is also rather redundant.
You should simply use isset($REQUEST['Section']), then test for that predefined variable in the switch.

Aside from that, yes, selecting all the columns could increase time taken.
Still, for such a small script, there really isnt all that much need to optimise.

darthmahon

Just tried it, seems to work ok.

Oh, never knew using * slowed it down? Will name them from now on.

How do you check how long a query took, apart from manually timing it? Is there are function in php to do this?

Thanks all for your help 🙂

Gary_King

Originally posted by darthmahon
Just tried it, seems to work ok.

Oh, never knew using * slowed it down? Will name them from now on.

How do you check how long a query took, apart from manually timing it? Is there are function in php to do this?

Thanks all for your help 🙂

If you are using SELECT FROM for a table that always changes columns, then just stick with SELECT FROM. If you are getting something very specific, then use SELECT col1 FROM. I don't suggest to use SELECT col1, col2, ... FROM for lots of columns, too much typing anyhow 🙂 (I'm talking like, 15+ or so cols).

midget

Just think the column list that provides the * functionality is not comming from heaven...

There are internal DB functions that understand your * as:

Go on..

Lets look for the fields in this table(s)... ok done... Now lets load them in an array/vector or whatever and keep them in memory ready to use...

All this work to maybe to use 2 of 20 fields???

Definitively * SLOWS everything... so you better type fields names... that's why many of us are paid for sometimes 🙂

midget