Make Code More Efficient?

obrienkev

Hi all,

How can I make the below more efficient? I need to add many more queries so is there any simpler way of doing this - maybe arrays?

        

// declare variables
$id = $_GET['id'];
$id2 = $_GET['id2'];
$id3 = $_GET['id3'];
$id4 = $_GET['id4'];

// check which file selected
switch(TRUE)
{
case ('$id'):
    $query = "SELECT name, type, size, content ".
                 "FROM centralised_archive.mergers ".
                 "WHERE id = '$id'";
                 break;

case ('$id2'):
    $query = "SELECT name, type, size, content ".
                 "FROM centralised_archive.rights ".
                 "WHERE id = '$id2'";
                 break;    

case ('$id3'):
    $query = "SELECT name, type, size, content ".
                 "FROM centralised_archive.spin_offs ".
                 "WHERE id = '$id3'";
                 break;

case ('$id4'):
    $query = "SELECT name, type, size, content ".
                 "FROM centralised_archive.splits ".
                 "WHERE id = '$id4'";
                 break;

default:
    echo 'None selected';
              break;                  

}

How is this done?

Thanks.

Kevin.

_theworks

How can I make the below more efficient? I need to add many more queries so is there any simpler way of doing this - maybe arrays?

what code?

thorpe

hehe

Roger_Ramjet

Not much more efficient than a Switch statement - plus you can just extend it anytime you need without any changes to the logic, unlike nested ifs.

Weedpacket

Yes, arrays.

Ravenous

// declare variables
$id = $_GET['id'];
$id2 = $_GET['id2'];
$id3 = $_GET['id3'];
$id4 = $_GET['id4'];
$query = "SELECT name, type, size, content FROM centralised_archive.";

// check which file selected
switch(TRUE)
{
case ('$id'):
    $query .= "mergers WHERE id = '$id'";
    break;

case ('$id2'):
    $query .= "rights WHERE id = '$id2'";
    break;    

case ('$id3'):
    $query .= "spin_offs WHERE id = '$id3'";
    break;

case ('$id4'):
    $query .= "splits WHERE id = '$id4'";
    break;

default:
    echo 'None selected';
    break;                            

}

Not more efficient, but less code.

Shrike

The inefficiency is in the switch statement, so is it possible to rethink the design so only one parameter gets passed to the script? What is the purpose of id1, id2 , id3 and so forth?

Ravenous

Rework the calling previous script so as to get something like this URI: index.php?id=SOME_ID&table=SOME_TABLE

// check which file selected
if(isset($_GET['id']) && isset($_GET['table']))
     $query = "SELECT name, type, size, content FROM centralised_archive.".$_GET['table']." WHERE id = '".$_GET['id']."'";
else
     echo 'None selected';

That would use the switch statement in a more traditional format. Plus it cuts out the unneccesary assignment statements. The point is that you have a lot of redundant code. Cut it out of the script and save yourself a lot of condition testing.

Weedpacket

Ravenous wrote:
The point is that you have a lot of redundant code.

Exactly; if you find yourself doing something over and over like this then there must be a better way that lets you do something once, and have the program do all the repetition. (Doing something over and over is something that computers are very good at.)