5

While asking another questions about PDO queries, I've been told that saving my PDO connection object as global to use it in my various functions that call queries to my database is generally bad practice.

Here is how I generally use my PDO object:

function somefunction(){
    global $pdo;

    $statement = $pdo->prepare("some query");
    $statement->execute();
}

The arguments I've read are more about code maintenance and debugging, where it's hard to trace who modified the PDO object and where in the code it would be. Other people just simply reject using global variables for storing the PDO object, but can't really explain why global variables are a bad approach.

However, for a small-medium projects with only one database, is there really a disadvantage to using a global variable? I usually have my connection script and my functions script separately, where the functions script will require_once() the connection script, where my PDO object is created. In this way, my connection is always established and all modifications to the PDO object are done in my connection script.

Is there any fundamental flaws in using this approach?

Prusprus
  • 7,987
  • 9
  • 42
  • 57
  • 1
    Please see http://stackoverflow.com/questions/12445972/stop-using-global-in-php – Achrome Oct 12 '13 at 18:28
  • 2
    Consider that you are building a new house. You would expect for the electrician to run all the wiring in the walls, so that you can have pretty sockets wherever you would like. But now imagine that the electrician tells you: "Instead of running all the wiring in the walls, I'm just going to use meters and meters of extension cords. I mean, what's so wrong with that? You still get electricity." – Mark Oct 12 '13 at 18:29

2 Answers2

12

Is there any fundamental flaws in using this approach?

The very first thing you have to understand, is that $pdo is a part of storage logic. That means, it should be only used inside classes that do abstract data access, be it a SQL table or a collection.

Let's look at your code,

function somefunction(){
    global $pdo;

    $statement = $pdo->prepare("some query");
    $statement->execute();
}

What if you want to switch from MySQL to Mongo/MSSQL/PgSQL, in future? Then you will have to rewrite a lot of code.

And for each database vendor, you will have to create a separated file with different variable. Just like this

function somefunction(){
    global $mongo;
    return $mongo->fetch(...);
}

By using a global state, you end up with mass code duplication, because you cannot pass parameters and thus cannot change function's behavior at runtime.

Now let's look at this,

function somefunction($pdo){
    $statement = $pdo->prepare("some query");
    $statement->execute();
}

Here, $pdo is passed as an argument, thus there's no global state. But the problem still remains, you end up violating the Single-Responsibility Principle

If you really want something that is maintainable, clean and very readable, you'd better stick with DataMappers. Here's an example,

$pdo = new PDO(...);

$mapper = new MySQL_DataMapper($pdo);
$stuff = $mapper->fetchUserById($_SESSION['id'])    

var_dump($stuff); // Array(...)

// The class itself, it should look like this
class MySQL_DataMapper
{
    private $table = 'some_table';

    private $pdo;

    public function __construct($pdo)
    {
        $this->pdo = $pdo;
    }

    public function fetchUserById($id)
    {
        $query = "SELECT * FROM `{$this->table}` WHERE `id` =:id";
        $stmt = $this->pdo->prepare($query);

        $stmt->execute(array(
           ':id' => $id
        ));

        return $stmt->fetch();
    }
}

Conclusion

  • It doesn't really matter if your project is small or large, you should always avoid global state in all it forms (global variables, static classes, Singletons) - For the sake of code maintainability

  • You have to remember, that $pdo is not a part of your business logic. Its a part of storage logic. That means, before you even start doing something with business logic, like heavy computations, you should really abstract table access (including CRUD operations)

  • The bridge that brings together your data access abstraction and computation logic is usually called Service

  • You should always pass the things function's need as parameters

  • You'd better stop worrying about your code and start thinking about abstraction layers.

  • And finally, before you even start doing any stuff, you'd firstly initialize all your services in bootstrap.php and then start querying storage according to user's input ($_POST or $_GET).

Just like,

public function indexAction()
{
    $id = $_POST['id']; // That could be $this->request->getPost('id')
    $result = $this->dataMapper->fetchById($id);

    return print_r($result, true);
}
Yang
  • 8,580
  • 8
  • 33
  • 58
  • I'm not sure this is true. Part of the point of PDO was to signularize database connection. Re-opening the PDO in non-global situations (and Linus forbid private) can result in needless opening-non-closing connections, and can lead to some serious overhead drawbacks. A blanket statement like "avoid global state in all its forms" completely ignores that the global state exists for a purpose. A managed pdo connection is one of those purposes. Further, maintaining the pdo connection in a single location makes it significantly easier to update when database architecture changes later. – lilHar Oct 27 '18 at 17:41
  • @liljoshu All handlers share the same database connection, the connection doesn't get established each time a passing occurs. So what's your point here again? – Yang Oct 28 '18 at 10:37
  • All handlers that use the same PDO variable use the same connection. If you constantly re-create PDOs, you're establishing multiple connections. Basically, each time you do "new PDO", you're creating a new connection (it'd be really weird if you weren't, considering each PDO contains its own connection string.) However, unlike mysqli, you don't create a new connection for each query, just each pdo. Which means, to save connections, you want to use one PDO global per database (or if you want some changed connection variable). – lilHar Oct 29 '18 at 15:29
  • 1
    @liljoshu In case multiple database connection needed, that would require another solution, which won't be like the one mentioned in the post. In terms of writing real-world applications, the very known and long established fact is that global state makes application state unpredictable and very hard to test in isolation, so it's recommended to avoid it at any cost. Also, there's a trick to make PDO connection to be connected on demand, which can be later injected into service locator, see here: https://pastebin.com/wdUJJWmy – Yang Oct 29 '18 at 18:53
  • If your problem is simply predictability, makes just as much sense to throw the PDO in $GLOBALS["my_codespace_identifier"]["Primary_PDO_connetion"] (which, to be fair, is generally better than using global). Although the pastebin you shared is a nice alternative if you're avoiding $GLOBALS for some crazy reason. Also in terms of writing real world applications, the actual very known and long established fact is there are no established facts, and it's better not to assume as such. – lilHar Oct 30 '18 at 00:04
  • @liljoshu **First of all**, if a variable (i.e dependency) has not been injected, then it comes from the global scope. Doesn't matter if author uses `global $var` or static method or `$GLOBALS` - those ones are the same sort of **global state**. You probably never followed Test-Driver-Development nor tried writing unit tests. – Yang Oct 30 '18 at 12:40
  • **Second**, predictability isn't a problem when one dependency being managed. This becomes a problem when managing nearly 10 services that constantly change their state. Just to name few - db, cache, session handler, config, auth, request, response, validator, translator and when one state depends on another. Just try writing large and complex web-application and you'll get this point. – Yang Oct 30 '18 at 12:41
  • When I mentioned so called established facts, I meant good practices and proven solutions to known problems. Most of them are called *patterns*. Also, I'll name few ones what most frameworks follow and implement - Routing/Dispatching, MVC-like structure, services – Yang Oct 30 '18 at 12:43
  • First off, you'd be wrong in your assumptions about me. Further, $GLOBALS vs global do have a key difference n reliability, specifically $GLOBALS are all known globals, while a variable with the global key state may be viewn and not immedeiately known it's a global, causing undpreditable edits. This is why global designator is unpreditable, while $GLOLBALS is much more reliable. They are quite different in practice, if functionally similar. Further, I have worked in my environments that used unit tests and TDD, and others that didn't. – lilHar Oct 30 '18 at 17:51
  • Secondly, "good practices" in one field are worst practices in another. I have found over my (very long) development career (I got started on punch cards). For example, gotos are hated across most of development, but absolutely vital when speed is of the essence (for example, game development or rocket guidance). Your assertion that your "established facts" are "good practices" only applies to the fields you've worked in. In other fields, what your'e suggesting is very much throwing the baby out with the bathwater, and is horrible for rapid development and prototyping proof-of-concepts. – lilHar Oct 30 '18 at 17:54
  • To put it simply, the only real drawback of the original poster's proposal in their stated scenario (small-to-medium with only one DB), is that the global keyword can easily be lost track of, an issue that is easily remedied by putting it $GLOBALS, which is fully maintainable, especially if maintained along with other such variables in a central config file. (And even if not, it's nothing that would cause creating it as a service to be any more beneficial than the ability to ```grep $GLOBALS``` ) – lilHar Oct 30 '18 at 18:15
  • @liljoshu Finally got your point. You're saying there's nothing wrong with putting prepared object in global container (`$GLOBALS` - global container) and later accessing it in methods. You know, what you're suggesting is called *Registry pattern* and its known anti-pattern. Just google for "why registry is anti-pattern" to find out why. What I'm suggesting is called *Dependency Injection* and its been there for years – Yang Oct 30 '18 at 20:35
  • You're assuming that dependency injection is something that's being worried about. This is not the case is most single-db small-project scenarios, as often the entire project falls in the sizeof what large enterprise projects would call a small code segment. The point is, for the original poster's scenario, you're trying to insert large-scale business requirements which is counter-productive, especially when limited dev time is a serious concern. You're taking _one_ coding paradigm and assuming it must hold true cross-spectrum. It doesn't. – lilHar Oct 30 '18 at 20:44
  • @liljoshu A developer whose time is limited shouldn't try to re-invent the wheel. He totally must use modern frameworks, where he shouldn't be bothered about how database object is being instantiated or passed around. Instead he should type several commands to generate CRUD where everything is handled for him internally. Also please take a look at this question: https://softwareengineering.stackexchange.com/questions/148108/why-is-global-state-so-evil – Yang Oct 31 '18 at 14:24
  • Well, there's your problem right there. You're relying on frameworks. The dumbest coding "tool" ever created. Frameworks eschew a million useful tools in an effort to make coding less effecient. As a wise dev once told me, "Frameworks are for devs that misspelled library." Please take a look at this [article.](http://tomasp.net/blog/2015/library-frameworks/) – lilHar Oct 31 '18 at 18:13
  • More seriously, there are primarily two situations where frameworks make sense; 1: Coding for a company large enough to where it needs to be broken up by anti-trust laws and the company has an internal framework to maintain consistancy. 2. Any other project where all the devs _only_ know how to work in that framework already, probably because they split off of a situation in case 1. Outside of frameworks, $GLOBALS are a lifesaver in many circumstances, and effectively form the backbone of a project's own 'framework'. Storing PDO in $GLOBALS is one such case. – lilHar Oct 31 '18 at 18:47
  • 1
    Also, has once been said, "Those afraid to reinvent the wheel would try to slap wagon wheels on a jet's landing gear." – lilHar Oct 31 '18 at 18:50
3

For a tiny tiny project it won't hurt much to use global. When the project size starts growing is when things start to get bad.

if your somefunction() had 300 lines in it using 5 global variables, someone looking at your code wouldn't know that the function uses outside variables unless they went through it looking for global variables.

Plus, it's so easy to not use global variables...so why do it

function somefunction( PDO $pdo ){
    $statement = $pdo->prepare("some query");
    $statement->execute();
}

EDIT:

show_profile.php is your controller. Your controller collects all the data for the view and loads the view. This is a very simplified version.

require( 'functions.php' );
$db = new PDO();
$user = get_user( $db, $user_id );
require( 'view.php' );
Galen
  • 29,976
  • 9
  • 71
  • 89
  • Well the way I see it, I have my pages that are called by the users, such as show_profile.php or update_settings.php. These pages call the functions that are in my script pages, that connect to the database. In my eyes, this separates the content structure from the compute logic that takes to connect and retrieve/update the database data. Having my pages to call the function with $pdo in parameter means that they must in a way deal with the PDO object and as such, this blurs the line between my user pages and my database pages. Or is this a wrong way of seing it? – Prusprus Oct 12 '13 at 18:50
  • @Prusprus - show_profile.php is your controller. its supposed to create a database connection. load necessary functions, etc. then load the view. see my edit. – Galen Oct 12 '13 at 20:13
  • Galen already shown a simple way to avoid globals in functions . I have shown in my answer one of the OO approaches I use anyway . – Uours Oct 12 '13 at 21:26