Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a web application where I connect with a MySQL database using PDO and in some scripts there are a lot of queries one after the other which don't necessarily concern the same tables. i.e I will execute a SELECT statement in one, and then depending on some values I will UPDATE another and finally go and DELETE from another one.

The thing is I only recently learned PHP and MySQL and because I wasn't sure and wanted to be careful and to find easily any problems (and a little because I am a little ocd and anal about silly things like uniformity and coding style) in every query I used the following format

try {
    $statement = "
        UPDATE/SELECT   ...
        FROM/SET    ...
        WHERE   ...";
    $query = $dbcnx->prepare($statement);
    $flag = $query->execute();
}
catch (PDOException $e) {
    $errorMsg = "...";
    error_log($errorMsg,3,'../../xxx.log');
    $response = ...;
    $dbcnx->null;
    return $response;
}
$result = $query->fetch/fetchAll/fetcColumn...

so I could find where any problem would occur (try/catch) and to be safe against injections and invalid characters (prepare) (I had some personal checks but I am pretty sure that a function specifically made for would be better).

When I had one or two queries it was fine but after the code grew it became a little too much code for little action/substance (like 16 lines for one query...)

So I would like some advise. How do I make my code more manageable? Is there some fundamental error in my logic on structure (the way I wrote it)? Is there some rule for using try/catch? Is it more for developing and debugging and afterwards you can remove some blocks of it? Is prepare enough for rudimentary security on queries?

I was thinking of making a function of just this block of code and calling it with the statement as a parameter. So I would just 'type' the query in the main body and then call the function where it would be prepared, executed and then return the result. Of course then I would always use fetchAll and would return an associative array but I think as long the datasets are small the memory usage would be fine (in any case in nowadays systems it should take A LOT to notice a difference, I think)...

EDIT: I was told about abstraction layer, which I admit I am a little fuzzy about. Basically it is a class which is instantiated every time the script/function runs and wrap the functions used repeatedly (along with the checking I do) inside it?

share|improve this question
add comment

1 Answer

You're on the right track, what you're looking for is a DAL (Data Abstraction Layer), a class that represents a database connection.
This will allow you to move your common code into the class so you can simply construct your SQL statement and pass it in, with the method returning the expected data.

At a minimum you'd typically have, but aren't limited to:

  • Constructor (__construct) to create a new database connection using the relevant permissions.
  • Method to run your select statement (after input sanitization), returning a result set.
  • Method to run statements that don't return a result set.
  • Destructor (__destruct) to clean up and free any resources.

This would give you the following advantages:

  • Code reuse: only 1 copy of logging, connection initialization code, in 1 place.
  • Code encapsulation: The details are hidden behind a public interface so you can easily fix bugs and add new features without breaking existing SQL calls.
  • Pass in a complete SQL statement for centralized execution of SQL code.
  • Optionally pass in a parameter to specify calling Fetch/FetchAll/FetchColumn.
  • Easier debugging since your class will very quickly become very reliable as it gets heavily used and bugs are noticed and fixed.
  • Logging would be done automatically whenever you ran an SQL statement.
  • Easily add additional methods, such as input validation/sanitization.
  • Additional methods for specific purposes depending on your requirements.
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.