Page 1 of 1

What is the best practice

Posted: Sun Jun 29, 2008 4:29 pm
by RikComery
Hi. I have a general question, which should be easy to answer.

I have been updating a group of PHP websites that were build by someone else. In all of them, there are many pages that have forms, that when submitted, open a php page with some query to a mysql database. In each case, the query is in a separate php file.

My question is this. Is it best to have every form action point to a different php file, or is it acceptable to have a single "Database Functions" file which contains different actions.

For example, is this acceptable?

Code: Select all

if(isset($_POST["action"])) {
  $action = $_POST["action"];
  if($action == "writeSomething") {
    // mysql query that writes to the database table
  } else if($action == "readSomething") {
    // mysql query that reads something from a database
  } else if($action == "updateSomething") {
    // mysql query that updates something in a database
  }
} else {
  print "Sorry, no action was supplied";
}
The above seems okay to me, but just thought i should check in case there is some security risk.

Thanks

Re: What is the best practice

Posted: Sun Jun 29, 2008 4:40 pm
by califdon
I am not an expert in this matter, but I don't see anything wrong with the example you gave. I don't think there's any increased security risk. As a general programming style question, though, there may be reasons to avoid excessively lengthy scripts that pack too many functions and conditionals that must be evaluated by the server every time the script is called. If you are dealing with old scripts, they may not employ good programming practices--I know that my old scripts, many of which are still in use, embarrass me now when I read them, even though they run properly (at least most of them)! :roll:

Re: What is the best practice

Posted: Sun Jun 29, 2008 7:45 pm
by Ollie Saunders
There's no security risk. But you could express your code better. This...

Code: Select all

 if($action == "writeSomething") {
    // mysql query that writes to the database table
  } else if($action == "readSomething") {
    // mysql query that reads something from a database
  } else if($action == "updateSomething") {
    // mysql query that updates something in a database
  }
...demands that you add or remove logic if you want to make a change. A better solution might be something like this:

Code: Select all

$action = "databaseAction$action";
if (!is_callable($action)) {
   throw new Exception('Unhandled action');
}
$action();