What are the advantages to passing a global DB connection into each function of a model?

Question

I am working with an older code base that passed a db connection into most functions in each class of the models. The db connection is created as a global and passed everywhere in the application:

$user = new User();
$user->loadById($db, $userId);

What advantages do we get by doing this vs a single connection the entire model inherits similar to the way most frameworks currently work?

Any insight would be very helpful.

Full Disclosure: I asked this question this way because this is how we do it at work. I don't like that we pass around the DB connection. I am trying to find a proponent of this method to see if my mind can be changed. That is why I tried to sway the discussion to the PRO side of this conversation without being blocked as a bad question. And it worked. I didn't get banned, but the great StackOverflow community didn't let me down. It appears I'm not out in left field with how I think about this issue.

Answer 1

The main advantage is: it's easier. As in, it's the simplest thing to do, because as a result, you have no application architecture. You're grasping at things from everywhere and anywhere because you have no idea how to get them otherwise, and this makes for very poor maintainability. Guess what happens 5 years down the line with this sort of codebase? Massive legacy technical debt, and it's very likely your developers aren't using Object Oriented Programming - more likely shoving procedural code in classes.

I'm not going to bother explaining about global state, because there's already a fantastic answer that already exists on programmers. A small excerpt:

Very briefly, it makes program state unpredictable.

To elaborate, imagine you have a couple of objects that both use the same global variable. Assuming you're not using a source of randomness anywhere within either module, then the output of a particular method can be predicted (and therefore tested) if the state of the system is known before you execute the method.

However, if a method in one of the objects triggers a side effect which changes the value of the shared global state, then you no longer know what the starting state is when you execute a method in the other object. You can now no longer predict what output you'll get when you execute the method, and therefore you can't test it.

You'll find some developers do this purely out of laziness or a lack of knowledge / understanding of the basic concepts of SOLID. If you access global state (like the database), then the beautiful, isolated class you are currently writing that theoretically can be handed off to any other developer, and also tested in it's own right, is now coupled to this object somewhere off in the clouds.

As well as the above, you're making a liar of your object API. Each object should, via it's constructor / method signatures, specify exactly the required external objects with which it requires to function. This allows:

• Your object to have a definitive API for it's usage
• Future developers can see exactly what is required for this object to function from the constructor / method signatures
• Everything passed in via Dependency Injection (the posh word for passing in parameters, basically), can be 'mocked out' for testability
• Developers don't need to read through your code to find out what other objects are required, because of point 2
• You aren't accessing something that can be changed by something else, somewhere else, and make debugging a nightmare

Your code should not be brittle. You should be completely confident in making changes somewhere in a massive codebase, without worrying about breaking something somewhere else. Your unit tests will cover this. I highly recommend reading The Clean Coder as it elaborates on some of these concepts.

AlmaDO has a picture of a good image about Singletons, which are basically objects that return a single instance of an object, like a database or logger. So if you request a new database from it, you either get a new one or just get back the one that already exists. In the traditional request / response / dead context, this is completely unecessary. If you're having a very long-running process, maybe this might be necessary, especially in other languages, but as a general rule of thumb in PHP; unless you're running a PHP web socket server or similar, DI is a much better way to go for maintainability.

This is exactly the same as calling StaticObject::Database, again - something that can be accessed from anywhere.

This is a really good post on Singletons in PHP, and how they're not at all needed - that post also has a lot of useful links in it further down.

Basically - don't be lazy and grasp SOLID. There's a reason it exists, and it's certainly not just for PHP, either. The main reason people do it is because they don't know better and it's easier, and easier isn't always the best way.

---

Also, a few additional clarifications. We don't have "models". We have a model, which is a layer. This is a massive misinterpretation of 'MVC', which we don't even actually have in PHP ('classical mvc'). We have the separation of concerns and that's it.

• Your "models" are likely, services, domain objects, entities. More info on what the model is

• Your user should not know about the database. It should be an Entity. Active Record Pattern is not the way forward. Take a look at DataMapper. Your repository would use the database object and return an array of User objects. Contextually as well, a user having access to the database makes no sense.

• Most frameworks do not work by having access to the database in entities - this isn't PHP 4 any more, it's progressed a lot since then ;-) I highly recommend moving away from CakePHP (it's not MVC in any way) and take a look at a framework like Symfony, which still has it's flaws - knowing the flaws of your chosen framework is very important