0

Problem

I'm using $GLOBALS[] variable to define names of tables from DB in my php files to make it simple changing names of tables and for general mobility/comfort. BUT..

I've heard that using $GLOBALS[] is SOMEHOW bad or something..


So, it's happening like this:

Tables in Database

Users
Orders
Products

Actually there are 10-20 tables...

global_vars.php

<?php

    $GLOBALS['t_users'] = 'Users';
    $GLOBALS['t_users'] = 'Orders';
    $GLOBALS['t_users'] = 'Products';

    //...
    //etc.

?>

Now, when I need to access DB from different pages of website and with different purposes I do it like so:

function GetUsers(){
    $sql = "SELECT * FROM $GLOBALS[t_users]";
    // ...execute
}
function Get_OneUser($id){
    $param['id'=>$id];
    $sql = "SELECT * FROM $GLOBALS[t_users] WHERE id=:id";
    // ...etc
}
function Get_Orders(){
    $sql = "SELECT * FROM $GLOBALS[t_orders]";
    // ...etc
}
function Get_OrdersB(){
    $sql = "SELECT * FROM $GLOBALS[t_orders] WHERE id=:id";
    // ...etc
}
function Get_Products(){
    $sql = "SELECT * FROM $GLOBALS[t_products]";
    // ...etc
}
//   -- AND SO ON, AND SO ON........

Imagine, one day I will need to rename tables/change database. Then (with this structure) I can only change one line in global_vars.php.. It will be perfect!


General Question

What could be a better way to make it?

Why this is/isn't good/bad? Help me figure it out! Thanks!

Max Maximilian
  • 531
  • 2
  • 17

2 Answers2

2

This has been asked loads of times here on StackExchange!

Have a look at the answer here, this will explain why.

Are global variables in PHP considered bad practice? If so, why?

Read up on dependency injection (use Pimple!). Using dependency injection, you can define everything in one place, and pass the required stuff in therough the constructor, or using the setters in the class. a DIC (dependency injection container) can also create factory methods for setting this stuff. So you get the benefit you are looking for, with less of the risk.

delboy1978uk
  • 10,948
  • 2
  • 14
  • 31
  • `global` is different from `$GLOBALS[]`, isn't it? `global` will loose the scope inside my functions, and `$GLOBALS[]` always is accessible (because of this I've selected `$GLOBALS[]` and not `global`) – Max Maximilian May 26 '17 at 14:08
  • 1
    You use `global $whatever` from inside a class in order to say you are using a variable from the global namespace. – delboy1978uk May 26 '17 at 14:09
  • @delboy1978uk as good practice, never use `global`s variables, that is why OOP was introduced to PHP, it "fixes" a lot of logical problems... If you are correctly developing your application, you will (almost) never use a `global` variable in any OOP language. – matiaslauriti May 26 '17 at 14:12
  • You don't need to tell me! ;-) Just clarifying for him what it does. – delboy1978uk May 26 '17 at 14:13
  • It's not terribly reasonable to expect your table names to change. Coding a feature to anticipate that is almost certainly a waste of time. That said, use [constants](http://php.net/manual/en/function.define.php) instead of global variables, or better, [Dependency Injection](https://en.wikipedia.org/wiki/Dependency_injection). – Alex Howansky May 26 '17 at 14:17
2

TL;DR: yes, but it depends what you want to improve.

The first issue is the global state. In PHP the $GLOBALS belongs to the group constructs, that are called "superglobals" and the issues they create are basically the same as with global variables.

The general idea of keeping the table names in a separate configuration file is a good one, but would suggest to structure it all a bit differently:

  • the SQL should go in the data mappers
  • initialization of data mappers should be done via factory or by using DI Container (are proper one, like Auryn or Symfony DI, and not some shitty service locator like Pimple)
  • pass the configuration to the mapper, through constructor as a parameter

Update

Data mappers are classes, that are responsible for interacting with persistence (usually database). The implementation details may vary, but the way I implement them, the usage ends up looking kinda like this:

$book = new Book;
$book->setId(51);

$mapper = new BookMapper($pdo, $config);
$mapper->fetch($book);

if ($book->getReleaseDate() < TWO_MONTHS_AGO) {
    $book->setDiscountPercents(30);
}

$mapper->store($book);

For more "practical" code, you can try looking at this class.

tereško
  • 56,151
  • 24
  • 92
  • 147