How can I avoid repeated declaration of variables as 'global'

PHP
1

I need to extract about 30 variables from MySQL for use in calculations. The calculations aren’t all that complicated, but I can’t tell in advance which variables I’ll need, so it seems best to extract them all.

I’m using a PHP function, so I started by declaring all the variables as ‘global’:-

	function calcBandbCost($setYr, $rate) {
		global $bb_cost, $yr_id, $w_rate_1, $w_rate_2, $s_rate, $o_rate, $x_rate, $t_rate, $c2_rate, $c1_rate, $b_rate, $t_age, $c2_age, $c1_age, $a_age, $cot_use, $cot_min, $min_dep, $max_pers, $next_bk_ref, $next_guest_id, $adv_days, $one_night_sup, $single_sup, $four_disc, $pp_dep, $pp_full, $first_date, $last_date, $link;
...
// stuff to do MySQL query, extracting values for all these variables
...
// stuff to do the calculations
...
}

I almost certainly won’t need all those to be accessible outside the function, so I should be able to pare the list down in due course.

Then I realised I’d need to call most of these variables at other points in the application, so I split out the MySQL call into a function of its own. But now I have to declare all those variables as ‘global’ in the MySQL call function in order to have them available for the calculation function (and for other functions too).

So now (for the moment, anyway) I’m declaring 30 or more variables as ‘global’ in two functions only a few lines apart in the file.


//	Get rates from DB:
	function getRates($link, $setYr, $rate) {
		global $bb_cost, $yr_id, $w_rate_1, $w_rate_2, $s_rate, $o_rate, $x_rate, $t_rate, $c2_rate, $c1_rate, $b_rate, $t_age, $c2_age, $c1_age, $a_age, $cot_use, $cot_min, $min_dep, $max_pers, $next_bk_ref, $next_guest_id, $adv_days, $one_night_sup, $single_sup, $four_disc, $pp_dep, $pp_full, $first_date, $last_date, $link;
		global $nights, $adults, $teens, $child2, $child1, $baby, $cot_req, $dep_amt;
		$query = "SELECT `yr_id`, `w_rate_1`, `w_rate_2`, `s_rate`, `o_rate`, `x_rate`, `t_rate`, `c2_rate`, `c1_rate`, `b_rate`, `t_age`, `c2_age`, `c1_age`, `a_age`, `cot_use`, `cot_min`, `min_dep`, `max_pers`,`adv_days`, `one_night_sup`, `single_sup`, `four_disc`, `pp_dep`, `pp_full` from rates WHERE yr_id = " . $setYr;
...
// stuff to do MySQL query
...
}
//---------------------------
//	Calculate B&B cost:
	function calcBandbCost($setYr, $rate) {
		global $bb_cost, $yr_id, $w_rate_1, $w_rate_2, $s_rate, $o_rate, $x_rate, $t_rate, $c2_rate, $c1_rate, $b_rate, $t_age, $c2_age, $c1_age, $a_age, $cot_use, $cot_min, $min_dep, $max_pers, $next_bk_ref, $next_guest_id, $adv_days, $one_night_sup, $single_sup, $four_disc, $pp_dep, $pp_full, $first_date, $last_date, $link;
		global $nights, $adults, $teens, $child2, $child1, $baby, $cot_req, $dep_amt;
...
// stuff to do the calculations
...
}

It works, but I feel there must be a better way, avoiding the repetition, which is only going to get worse when I want to use these variables in another function. I’m using procedural code. I suspect it may be easier with OOP but I’m not ready to go there yet.

I’ve considered saving them all to $_SESSION and extracting when required, but is there perhaps a better, recommended, way ?
Perhaps it will become blindingly obvious the moment I post this thread (not an unknown phenomenon).

2

I think this is a good case of using an array and maybe using $_SESSION (Just a long as there is no sensitive data), but definitely an array. An maybe an initialization file that you stick at the top of the page. An if you get advance in the future this would be case for a class.

I would also checkout : http://php.net/manual/en/function.implode.php it might help you out.

3

Ideally, your functions want to be passed all the data that they need to do their work, and to return all the required results. If a function relies on global variables (i.e. global state), it can have side effects on the rest of the program that are not readily apparent when calling the function. Changing a global variable in one place could easily break a function in another and would be harder to debug.

As Strider64 says, you could use arrays to group data. Your getRates function, for example, could return an array of all the rate information which can then be passed into other functions, such as calcBandbCost, that need the data to do their work.

Separating your DB access from your calculation function was a good move. In general, you want to avoid your functions doing too much and becoming too complicated. One sign that a function is doing too much is that it requires a lot of different inputs to do its job, and you should think about breaking up the functionality into separate, smaller procedures.

4

Thank you, Strider and fretburner. I’ll see what can be done with arrays. But there doesn’t seem a way around declaring all the MySQL varaibles as global at least once.

If a function relies on global variables (i.e. global state), it can have side effects on the rest of the program that are not readily apparent when calling the function. Changing a global variable in one place could easily break a function in another and would be harder to debug.

Because these particular variables are coming from a MySQL table they generally have the same value no matter what their scope. However, it’s a valid point.

I often find the problem the other way around, that a function doesn’t work because it can’t access a variable that has not been declared global. This has probably led me to declare some variables as global when it’s actually unnecessary, ‘just in case’.

5

Hi Tim,

Could you post some of the code that uses both of these functions? It might make it easier to give some more concrete suggestions as to how you could structure things to avoid using globals.

6

Object oriented programming is the way to go, you can cut way down on code and enforce stricter rules that are easier to understand.

I’d create an object which stores your variables in a private array as so:


class MyObj1 {
  private $Details = array();
  public function __construct(PDO $DBConnection, $setYr) { //enforces you to supply a pdo connection to use
    $this->Details = //pdo query result
  }
}

class MyObj2 {
  private $MyObj1;

  public function __construct(MyObj1 $MyObj1) { //enforces you to supply a n instance of MyObj1
    $this->MyObj1 = $MyObj1;
  }

  public function CalcBandB() {
    //use MyObj1 that was supplied to retrieve info needed...
  }
}

This is a horribly lazy example, but hopefully it will give you an idea. :slight_smile:

7

Thank you. I don’t doubt you’re right, but I’ve a job to do here, and I can’t take the time off right now to learn OOP. I’ll have to sooner or later, I know.

8

Easier said than done without posting more code than anyone could be expected to read. I have experimented with returning the results from the MySQL query as an array, and I’ve now realised that this DOES make it unnecessary to declare a whole lot of globals. The only reason for doing it was to make them available outside the function after it had run, which the array now does mush more elegantly. So I think my problem is pretty well solved.
Thank you for your help.
One of the great benefits of a forum is that in explaining one’s problem one is forced to (try to) think clearly about it !

9

That really needs to be cleaned up. A single global is a sign of poor programming but this takes the cake. That said sticking to the procedural nature of you code static variables could be used with an array or object. That isn’t really the best way to do it but much better gigen the procedural nature of what has been presented.


function foobar() {
  static $row;

  if(is_null($row)) {
    $row = array(); // could also be an stdclass or domain object
    // populate row with mysql data
  }

  return $row;

}

function bar() {
  $row = foobar();
  ...
}

The ideal way to do this would be to use a repository with some type of caching mechanism passing/injecting the dependency into other objects. Though that starts to get into OO w/ domain models and dependency injection which is a much more complex thing to set-up. Especially if you are not using some type of framework to manage dependency injection and all the standard patterns in a robust MVC framework.

I don’t quite understand the business logic associated with your code. Otherwise I would provide some direction to factoring it into classes.

10

Hi oddz,

That’s interesting, I wasn’t aware of static variables for procedural style code - you learn something new every day! In your example though, surely making $row static is unnecessary, as bar gets $row as the return value from foobar? Also, a static variable seems, for all intents and purposes, the same as a global variable.

11

Thank you. I must have read all the wrong books when I was learning PHP, as I was clearly told that to make variables available outside a function they’d have to be declared global within the function (and likewise to make variables declared outside the function available within). The Manual says much the same thing. That said I agree it becomes inelegant when there are more than a few.

I’ve taken on board what Strider and fretburner said previously, and by using an array I’ve cut right down on the globals.

Can you explain to me what’s so awful about one global, please ? I know there can be problems with values getting changed inadvertently through confusion with non-global vars of the same name. I just avoid using the same var name twice.

As fretburner says, a static variable seems much the same as a global.

12

Well… no

A static variable is only accessible within the routine which it is defined be it a function or method. Where as a global is available throughout the entire application – huge difference. One has a very narrow scope the other a wide one.

Well…

The only responsibility of foobar is to provide the data and make sure the logic to derive the data is only executed once. What is done with that data once fetched from foobar is entirely dependent on the needs of the application. This way several functions or methods have access to the foobar data through a consistent interface or in procedural terms the same function.

That’s interesting, I wasn’t aware of static variables for procedural style code - you learn something new every day!

Like I was saying this is not an ideal way to do it. However, when you’re stuck in the land of procedural programming it is an good solution to the alternatives. This problem can easily be solved in *better ways when using modern OOP patterns. So unless you have been dealing with procedural code lately (cough Drupal <8) it isn’t something that is important to know. Though even standard c\c++ provides static variables just like zend engine. The concept of static variables has merely been passed on from c to php.

13

Ah ok, sorry, I’d missed that bit about it being local to the function scope.

I also see why you’re using it now… basically as a cache. Interesting.

14

Yeah, I was hesitant to use the term cache due to confusing the op but that is exactly what it is. A procedural way to store state within the a individual function. In this instance using the static variable to cache the data.

15

Thank you, oddz. I can now see how to keep the ‘rates’ extracted from the DB as a static variable. Usefully I no longer have to extract them every time the function is run (though I expect they’ll be lost when the page is refreshed, as usual, unless saved to the SESSION). That has enabled me to dispense with a lot of globals.

This is what I’m working with at present, modelled on your suggestion:

//	Get rates from DB:
	function getRates($link, $setYr, $rate) {
		static $rates;

		if(empty($rates)) {
			$rates = array(); // not necessary ?
			$query = "SELECT `yr_id`, `w_rate_1`, `w_rate_2`, `s_rate`, `o_rate`, `x_rate`, `t_rate`, `c2_rate`, `c1_rate`, `b_rate`, `t_age`, `c2_age`, `c1_age`, `a_age`, `cot_use`, `cot_min`, `min_dep`, `max_pers`,`adv_days`, `one_night_sup`, `single_sup`, `four_disc`, `pp_dep`, `pp_full` from rates WHERE yr_id = " . $setYr;
			$result = mysqli_query($link, $query);
			if (mysqli_num_rows($result) == 1) {
				$row = mysqli_fetch_assoc($result);
				foreach ($row as $key =&gt; $value) {
					$rates[$key] = $value;
				}
			}
		}
		return $rates;
	}

//====================================================================
//	Calculate B&B cost:
	function calcBandbCost($link, $setYr, $rate) {
		global $nights, $adults, $teens, $child2, $child1, $baby, $cot_req; // user input: length of stay, number of people, etc

		$rates = getRates($link, $setYr, $rate);
		$bb_cost = 0;
		extract($rates);
//		echo 'Adult Rate is ' . $rate . '&lt;br /&gt;';
		$bb_cost += $nights * $$rate * $adults;

//	If one night only, add supplement to adult rate (£):
		if ($nights == 1) $bb_cost += $adults * $one_night_sup;

//	If single occupancy, apply supplement (%age)::
		if ($adults == 1) $bb_cost += $bb_cost * $single_sup/100;

//	If four or more adults, apply discount (£):
		if ($adults &gt;= 4) $bb_cost -= $adults * $four_disc;
		echo 'Adult cost = ' . $bb_cost . '&lt;br /&gt;';

//	Costs for teens, children, baby:
		$bb_cost += $nights * $teens * ceil($s_rate*$t_rate);
		$bb_cost += $nights * $child2 * ceil($s_rate*$c2_rate);
		$bb_cost += $nights * $child1 * ceil($s_rate*$c1_rate);
		$bb_cost += $nights * $baby * ceil($s_rate*$b_rate);

//	Add cot if required:
		echo 'Cot_req is ' . $cot_req . '&lt;br /&gt;';
		if ($cot_req == 'Yes') {
			$cot_fee = $nights * $cot_use;
			if ($cot_fee &lt; $cot_min) {
				$cot_fee = $cot_min;
			}
			$bb_cost += $cot_fee;
			echo 'Cot fee = ' . $cot_fee . '&lt;br /&gt;';
		}
		$bb_cost = number_format($bb_cost, 2);
		echo 'Total cost = ' . $bb_cost . '&lt;br /&gt;';
		$costs[] = $bb_cost;

//	Calculate deposit:
		if (ceil($bb_cost/10) &lt; $min_dep) {
			$dep_amt = $min_dep;
		} else {
			$dep_amt = ceil($bb_cost/10) &lt; $min_dep;
		}
		echo 'Deposit = ' . $dep_amt . '&lt;br /&gt;';
		$costs[] = $dep_amt;

//     Return B&B cost and deposit amount
		return $costs;
	}

I’m still unclear how I get half a dozen user entered variables out of the main script and into the ‘calcBandbCost()’ function without declaring them as globals. They could become arguments to the function, I suppose, but that seems equally ungainly. I could extract them from $_POST within the function, but then I’ll get other superfluous vars in the function, and I’ve already extracted them once in the main script.
I don’t yet see a better (shorter, quicker) way than declaring them as globals, nor why you regard doing that as poor programming. But I’m willing to learn.

16

The variables ($adults, $teens, $baby etc.) are closely related data and can be thought of as a unit (perhaps a booking or a party) and so I’d create a data structure to hold them. In OOP this would be a class, but we could always use an array as a substitute. This makes it the data much easier to pass into functions that need it.

It seems to me that by simplifying what needs to be passed around, the code that ties it all together could be quite neat and readable:


// No filtering or sanitizing shown, but you get the idea
$booking = array(
    'adults' => $_POST['adults'],
    'teens'  => $_POST['teens'],
    'child1' => $_POST['child1'],
    'child2' => $_POST['child2'],
    'baby'   => $_POST['baby'],
    'cot_req'=> $_POST['cot_req'],
    'nights' => $_POST['nights']
);

$con = getDB();
$rates = getRates($con, $setYr);
$costs = calcBandBCost($booking, $rate, $rates);
17

Thanks, fretburner, that looks like a huge improvement. I’ll work on it.

I’ve tried several times to get to grips with OOP, but real life always seems to get in the way.