lose the globals in functions

Question

I have a custom CMS system I have written in PHP. I'm almost finished turning all the old legacy mysql_ functions into PDO. However, I have a ton of functions in one file, there is no class wrapper. Just about 50 functions in the file that is really every function needed to run the CMS. Many of days ago, I got into the bad habit of using globals like so:

function getWidgets($widget_id){
 global $db, $BLOG_ID;
 $stmt = $db->prepare("SELECT * FROM widget_assoc WHERE bid=? AND aid=?");
 $stmt->execute(array($BLOG_ID, $widget_id));
 $matches = $stmt->rowCount();
 if($matches !== 0){
 for($i = 0; $path[$i] = $stmt->fetch(); $i++) ;
 array_pop($path);
 return $path;
 }
 }

Variables like $db and $BLOG_ID need to be constant, as many functions rely on these two vars (and a few more) I'm not sure the best way to go about this clean up job. can I just wrap the entire functions file into one class? would I have to change all my output vars from the functions to $this-> ?

I'm trying to find a painless way to remove all the globals without having to rewrite all the functions and templates that parse function outputs in the themes.

I've read a lot about how globals are bad lately but I cant seem to wrap my head around a simplified way to achieve this. It may not be possible without extensive over hauling of my code. That's why I'm here! Thanks.

EDIT: I use a config.php that is called in the header of every CMS page. This config file contains vars like $BLOG_ID = '12'; and is created dynamically on site build. These vars are available to all pages but to get them inside my functions I have to use global. I don't have any experience with classes and made a mistake of just housing many functions inside of one functions file with no classes.

• arkascha: Certainly using a class is a good point to start. But rarely one single class is a good idea. Classes are meant to describe a specific thing or feature, not simply all there is. Anyway, using a class you get rid of the globals. You do not even have to use object properties. If those values are really (pseudo) constant, then why not use class constants?
• Sonny King: Thanks @arkascha, I did not know you could use classes withut using the object properties. Can you point me to an example of using class constants?
• mario: There's nothing wrong with $db per se. That's habitually and widely done not out of mischiev, but because it does happen to suit the common case and not introduce witless abstractions. -- Now $BLOG_ID is something different altogether. If it amounts to a configuration setting or indeed constant, then get rid of it either way.
• arkascha: Just something like class myClass { const BLOG_ID = 15; }; $useCase = myClass::BLOG_ID; That way you don't polute the global name space.
• Sonny King: @arkascha works great until I try to assign BLOG_ID = $BLOG_ID.. it won't let me assign the const to an existing var? $BLOG_ID is being defined outside the class (before it)

Answer 1

There are different ways you can approach this problem, depending on how much time you want to spent doing it and how "well" you want to do it.

Leave it as it is

You can leave the method the way they are assuming that they are working correctly. As per your comment about not wanting to rewrite everything, this is probably your best approach.

At the end of the day, using global variables in all your functions is (in my opinion) no more or less bad practice than having a file containing 50 separate functions without classes.

Pass the globals in

If you change the global variables to function arguments instead it gives you the insight to be able to know exactly what the value of the variable is and where it comes from when you call your method, then you pass the variable into the method and remove the need for globals.

Use classes with dependency injection/inheritance

This is probably the "best" approach, but also the one that will take the longest to achieve. I'd suggest you do it anyway to be honest.

So assuming that your 50 method file contains a variety of methods for different purposes, you might decide you need 1, 2 or 3 base classes and 5-10 classes that have a purpose or role, for example you might have an abstract Base class that sets up your PDO (DB class) connection, then your Blog class (example) might extend the Base class. This way, the Blog class will inherit all of the external dependencies it needs to produce a Blog entry (external meaning that the Blog class may assume that its purpose is to retrieve, format and output a Blog post only - DB connection should be handled already).

A practical example might be like this:

/**
 * Handle your database connection, querying etc functions
 */
class DB {
 protected $_pdo;
 public function getPdo() {
 if (is_null($this->pdo)) {
 $this->_pdo = new PDO(...);
 }
 return $this->_pdo;
 }
 public function __construct() {
 return $this->getPdo();
 }
 public function query($sql, $binds = []) {
 // write a function that executes the $sql statement on the
 // PDO property and return the result. Use $binds if it is not
 // empty
 $eg = $this->getPdo()->prepare($sql);
 return $eg->execute((array) $binds);
}
/**
 * Create a basic framework for all purpose-classes to extend
 */
abstract class Base {
 /**
 * "DB" property might be broad here to cover other DBs or connection
 * methods (in theory)
 */
 protected $_db;
 public function __construct() {
 $this->_db = new DB;
 }
 public function db($sql, $binds) {
 return $this->_db->query($sql, $binds);
 }
 // insert other common methods here that all type-specific classes
 // can use
}

Now the implementation of a specific action/role:

class Blog extends Base {
 public function get($blogId = null) {
 // Basic error check
 if (empty($blogId)) {
 throw new UnexpectedValueException('Blog post ID was missing!');
 }
 return $this->db('SELECT * FROM `blogposts` WHERE blog_id = ?', $blogId);
 }
}

NOTE I haven't tested this, but now the principle is that the Blog class only contains the logic that is specific to a Blog post. Any formatting functions, security functions etc can either be in the Base class or in another auxiliary class that the Base class uses similarly to DB, e.g. a Formatter class.

You should be able to do this then:

<?php
# blogPost.php
# - Gets a blog post
require_once 'common.php'; // <--- include your class files, or an autoloader
// Instantiate the class for this role
$blog = new Blog;
// Get the blog post
$id = (isset($_GET['id'])) ? (int) $_GET['id'] : null;
$post = $blog->get($id);
// now other methods:
$post->toHTML(); // example - function might call a template file, insert the
 // DB results into it and output it to the browser

This is just a rough example, but shows how you could structure a set of classes to achieve the principle of a class having a single role, and methods having single purposes (e.g. "get a blog post by its ID").

This way, everything that extends Base will automatically have access to the database (via inheritance). You could add to your DB class some methods to provide a basic ORM if you wanted to try and remove the SQL from the implementation.

Make a single class for common values

Another short/quick option would be to create a single class that can serve anything common to all of the functions that will be in your included file, in this case the DB handler and the blog post ID. For example:

class Common {
 protected static $_db;
 protected static $_blogId;
 public function getDb() {
 if (is_null(static::$_db)) {
 static::$_db = new PDO(...);
 }
 return static::$_db;
 }
 public static function getBlogId() {
 return (int) static::$_blogId;
 }
 public static function setBlogId($id) {
 static::$_blogId = (int) $id;
 }
}

Now you just need to instantiate this class before you start calling your functions, and set the blog post ID (if you need it). The PDO connection will be created lazily whenever its required.

# functions.php
require_once 'common.php';
function getWidgets($widget_id) {
 $stmt = Common::getDb()->prepare('SELECT * FROM widget_assoc WHERE bid = ? AND aid = ?');
 $stmt->execute(array(Common::getBlogId(), $widget_id));
 $matches = $stmt->rowCount();
 if ($matches !== 0) {
 for($i = 0; $path[$i] = $stmt->fetch(); $i++);
 array_pop($path);
 return $path;
 }
}

Your only responsibility here would be to set the blog post ID on every page, eg:

# blogPost.php
require_once 'common.php';
// Manual dependency blog ID needs to be set before processing:
$blogId = isset($_GET['blog_id']) ? (int) $_GET['blog_id'] : null;
Common::setBlogId($blogId);
// now you call your processing methods and perform your logic flow

• Sonny King: Thank you for the abundance of information! I think for now, I will use your latter easier solution, then begin to rewrite everything into classes as I have time. I want to do it the right way. dont have much experience with classes but I better learn about it now to save me from trouble later as my application grows.

Answer 2

If you want to use variables at multiple place, you could create a singleton for those variable.

class Config
{
/**
 * @var Singleton The reference to *Singleton* instance of this class
 */
private static $instance;
public $db = 'db';
public $BLOG_id = 'id';
/**
 * Returns the *Singleton* instance of this class.
 *
 * @return Singleton The *Singleton* instance.
 */
public static function getInstance()
{
 if (null === static::$instance) {
 static::$instance = new static();
 }
 return static::$instance;
}
/**
 * Protected constructor to prevent creating a new instance of the
 * *Singleton* via the `new` operator from outside of this class.
 */
protected function __construct()
{
}
}
//To use it
$config = Config::getInstance();
$config->db;

• Sonny King: thanks, how would I use the var like $db in my functions? how would I call them?
• Sonny King: This does not work for me. I assume Singleton:: should be Config::? but I cant get it to output any data from the vars.
• Yann Thibodeau: Just changed it, there was an error. I tested it and it works as intended.
• Sonny King: works without error, but I still need $db and $BLOG_id to be defined from an existing $var. is this not possible? when I try it in your class, it crashes.