Jump to content

Recommended Posts

Hi guys!

I have some code like this, but everyone on the internet says that globals are wrong. Need your advice.

 

I have a function which tells me what was selected: section, category, page or user. According to that selection, code.php will include different pages. Is this wrong? If yes, how can accomplish something like this?

 

function find_selected_item_id() {

global $sel_sec;

global $sel_cat;

global $sel_pag;

global $sel_usr;

if (isset($_GET['sec'])) {

            $sel_sec = $_GET['sec'];

            $sel_cat = NULL;

    $sel_pag = NULL;

    $sel_usr = NULL;

        } elseif (isset($_GET['cat'])) {

            $sel_sec = NULL;

            $sel_cat = $_GET['cat'];

    $sel_pag = NULL;

    $sel_usr = NULL;

        } elseif (isset($_GET['pag'])){

            $sel_sec = NULL;

            $sel_cat = NULL;

    $sel_pag = $_GET['pag'];

    $sel_usr = NULL;

        } elseif (isset($_GET['usr'])){

    $sel_sec = NULL;

            $sel_cat = NULL;

    $sel_pag = NULL;

    $sel_usr = $_GET['usr'];

} else {

    $sel_sec = NULL;

            $sel_cat = NULL;

    $sel_pag = NULL;

    $sel_usr = NULL;

}

    }

 

content.php

 

<?php

    find_selected_item_id();

   

    if (isset($sel_sec)){ //a section was selected from the top-menu

$view = $_GET['view'];

$file = "views/{$view}.php";

include $file;

    }elseif (isset($sel_pag)){ //a page was selected from the pages_list

include('views/page.php');

    }elseif (isset($sel_cat)){ //a cateory was selected from the categories_list

include('views/users_list.php');

    }elseif (isset($sel_usr)){

include('views/user.php'); //a user/company was selected from the users_list

    } else{

include('modules/front_page.php'); //nothing selected, the module "front_page" is included

    }

?>

 

Link to comment
https://forums.phpfreaks.com/topic/172367-why-are-the-globals-wrong-need-help/
Share on other sites

I have some code like this, but everyone on the internet says that globals are wrong.

 

If globals are wrong how are we then supposed to program?

 

$var1 = 'hello';//global variable (can be accessed as: $GLOBALS['var1'])
$var2 = 'world';//global variable (can be accessed as: $GLOBALS['var2'])

 

..everyone on the internet says that globals are wrong..

 

I didn't.

So, it's nothing wrong with code?

 

I didn't say that. I do think that you may apply some more validation on input like: $sel_sec = $_GET['sec']; can use a good dosage of validation. I could pass anything to sec as your code doesn't check what it contains.

I have some code like this, but everyone on the internet says that globals are wrong.

By this they mean the use of defining variables as global within functions.

 

If I didn't see the code for your find_selected_item_id() function how would I know where the variable $sel_sec, $sel_cat, $sel_pag and $sel_usr are being defined? I would have to hunt through masses of code to find out.

Edit: Verbose version of the above post ^^^

 

Let me provide an example of why you don't write functions where you must know the variable names it uses internally for each specific piece of data it operates on. Suppose that all of php's built in functions where written this way (as far as your application is concerned there is no difference between a user written function and a built-in function), where you must know what main program global variables to put values into for each function before you call it and you must use unique variable names in each function that exists (and your user written functions could also not use any of the variables that php's functions are using) so that they won't interfere with each other (you need to be able to call any function in any order in any place in your code for it to be useful, unless you plan to spend a lot of time rewriting functions to make them work just because the situation where you are using them changes.) Would anyone be able to efficiently write code using php if its' functions worked this way?

 

On a smaller scale, after you have more than a few functions in any piece of code and have more than a few dozen lines of code, writing functions this way makes coding harder, not easier, because will spend more and more time just remembering (or looking up) what variables you need to use for each function rather than just calling the function with the values you currently need it to operate on as parameters in the function call.

 

Ok, so you managed to write an application this way that works because you managed to remember everything necessary for the amount of time it took you to write it, but how about a month from now when you have forgotten all the specific variable names that you setup for your functions to use and need to read through your code to make a change, a correction, or reuse parts of it. When you come across a function call, which of the two methods will be easier to figure out what data that function call is using at the time it was actually called?

 

And don't forget that once you start working as a programmer on a large project, broken up between multiple programmers (or simply other code that you have written at a different point in time), that you will soon get fired (or very frustrated with yourself) for writing code this way because it is impossible to write modular code that can be easily integrated with the code written by other programmers (or simply other code that you have written at a different point in time) when you write functions this way.

Edit: Verbose version of the above post ^^^

 

Let me provide an example of why you don't write functions where you must know the variable names it uses internally for each specific piece of data it operates on. Suppose that all of php's built in functions where written this way (as far as your application is concerned there is no difference between a user written function and a built-in function), where you must know what main program global variables to put values into for each function before you call it and you must use unique variable names in each function that exists (and your user written functions could also not use any of the variables that php's functions are using) so that they won't interfere with each other (you need to be able to call any function in any order in any place in your code for it to be useful, unless you plan to spend a lot of time rewriting functions to make them work just because the situation where you are using them changes.) Would anyone be able to efficiently write code using php if its' functions worked this way?

 

Ah the good old Assembler days put all registers on stack modify the registers call the values from stack in reverse order and place them back into their original registers ;)

I've never seen a good use for globals.  Functions have argument lists for a reason.

 

PHP is a language with many possibilities (including global and goto) and what's a curse for one may be a blessing for another.

 

Provide an example then.

 

For 'global' something like:

 

$oldErrorHandler = set_error_handler('error_handler');

function error_handler($errno, $errstr, $errfile = '', $errline = 0, $errcontext = array()) {
    global $oldErrorHandler;
    ..
}

 

For 'goto' command-line routines (cron, etc..). Quick-and-dirty ;)

By encapsulating it in an object.

 

<?php
abstract class ErrorHandler
{
private $_oldErrorHandler;
private $_useStack = false;

public function __construct($useStack = false)
{
	$this->_oldErrorHandler = set_error_handler(array($this, 'error'));
	$this->enableStack($useStack);
}

public function enableStack($enable = true)
{
	$this->_useStack = (bool) $enable;
	return $this;
}

public function getOldHandler()
{
	return $this->_oldErrorHandler;
}

public function error($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array())
{
	$this->_handleError($errno, $errstr, $errfile, $errline, $errcontext);

	if ($this->_useStack && $this->_oldErrorHandler !== null) {
		call_user_func_array($this->getOldHandler(), array($errno, $errstr, $errfile, $errline, $errcontext));
	}
}

abstract protected function _handleError($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array());
}

class ConcreteErrorHandler extends ErrorHandler
{
protected function _handleError($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array())
{
	echo 'someone screwed up: ' . $errstr . PHP_EOL;
}
}

class AnotherHandler extends ErrorHandler
{
protected function _handleError($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array())
{
	echo 'Error: ' . $errstr . PHP_EOL;
}
}

$handler = new ConcreteErrorHandler();
trigger_error('hello world', E_USER_WARNING);

$handler2 = new AnotherHandler(true);
trigger_error('hi world', E_USER_WARNING);

 

Output:

someone screwed up: hello world
Error: hi world
someone screwed up: hi world

By encapsulating it in an object.

 

<?php
abstract class ErrorHandler
{
private $_oldErrorHandler;
private $_useStack = false;

public function __construct($useStack = false)
{
	$this->_oldErrorHandler = set_error_handler(array($this, 'error'));
	$this->enableStack($useStack);
}

public function enableStack($enable = true)
{
	$this->_useStack = (bool) $enable;
	return $this;
}

public function getOldHandler()
{
	return $this->_oldErrorHandler;
}

public function error($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array())
{
	$this->_handleError($errno, $errstr, $errfile, $errline, $errcontext);

	if ($this->_useStack && $this->_oldErrorHandler !== null) {
		call_user_func_array($this->getOldHandler(), array($errno, $errstr, $errfile, $errline, $errcontext));
	}
}

abstract protected function _handleError($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array());
}

class ConcreteErrorHandler extends ErrorHandler
{
protected function _handleError($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array())
{
	echo 'someone screwed up: ' . $errstr . PHP_EOL;
}
}

class AnotherHandler extends ErrorHandler
{
protected function _handleError($errno, $errstr, $errfile = '', $errline = 0, array $errcontext = array())
{
	echo 'Error: ' . $errstr . PHP_EOL;
}
}

$handler = new ConcreteErrorHandler();
trigger_error('hello world', E_USER_WARNING);

$handler2 = new AnotherHandler(true);
trigger_error('hi world', E_USER_WARNING);

 

Output:

someone screwed up: hello world
Error: hi world
someone screwed up: hi world

 

Yes that is if you use an OOP approach but what for those that use the procedural approach?

That is why OOP is better than procedural. It enables you to encapsulate things. Even then, the fact that it's not possible using procedural code without using globals doesn't mean globals are good. It could also mean that the error handling is not properly implemented in PHP. It could for instance pass the old error reporter via argument.

Yes that is if you use an OOP approach but what for those that use the procedural approach?

 

function setErrorHandler($oldErrorHandler = null)
{
   if($oldErrorHandler)
   {
      //use the old info
      //set with create_function

      $customErrorHandler = create_function("$errno, $errstr, $errfile, $errline, $errcontext", /* function code */);
   }
   else
   {
      //do the same thing, only without utilizing the old handler (because it doesn't exist)
   }

   $newErrorHandler = set_error_handler($customErrorHandler);
   return $newErrorHandler;
}

$errorHandler = setErrorHandler($oldErrorHandler);

 

You use create_function to create two separate error handlers - one that relies on the old error handler, one that doesn't.  You then set whichever one is used as the system's new error handler and return it.

First of all I don't want to be an ass I may sound like one but that is purely the fault of the emotionless state of text.

Second I agree with pfmabismad on all discussed points.

Third too bad we always have to screw someone's thread while discussing. It would be much nicer if we could all sit around a table and drink a nice beer while we are at it.

 

Even then, the fact that it's not possible using procedural code without using globals doesn't mean globals are good.

 

And I'm not going to say globals are good but I like to use 'global' when I'm prototyping it shows me what variables a function needs (argument list) and which external variables it uses to achieve it's goal and may serve as a guideline for the eventual OO design. My previous employer used PHP to prototype pieces of the application and used Java for the eventual release. IMO doesn't PHP deserve such a discrimination. The same goes for those that call PHP a bad programming language because it has a GOTO statement or it's others so said flaws..

 

It could also mean that the error handling is not properly implemented in PHP. It could for instance pass the old error reporter via argument.

 

Mail Zend and tell them they are idiots and while you are at it tell them to remove GOTO and rewrite the namespaces implementation ;)

Wooow!

Things got pretty hot i here!

I just want to clarify few things:

1. I start learning PHP about a year ago and I code only during the night when the kids are sleeping. Just kidding!

2. I came to this website to learn from others' experience

3. I want to stick with procedural until I learn some basics for good programming.

 

About my code...

 

 

The function "find_selected_item_id()" tells me all the time what kind of view was selected from the top_menu and includes in the content.php pieces of codes accordingly. Then the same function tells me what item i selected from the code i just included, and then includes another piece of code or just display a page.

 

function display_top_menu(){
echo "<ul>";
        $section_set = get_all_visible_sections();
        while($section = mysql_fetch_array($section_set)){
            echo "<li><a href=\"index.php?sec={$section['id']}&view={$section['view_type']}\">" . $section['name'] . "</a></li>";
        }
echo "</ul>";
    }

 

 

 

content.php

<?php
    find_selected_item_id();
    
    if (isset($sel_sec)){ //a section was selected from the top-menu
$view = $_GET['view'];
$file = "views/{$view}.php";
include $file;
    }elseif (isset($sel_pag)){ //a page was selected from the pages_list
include('views/page.php');
    }elseif (isset($sel_cat)){ //a cateory was selected from the categories_list
include('views/users_list.php'); 
    }elseif (isset($sel_usr)){
include('views/user.php'); //a user/company was selected from the users_list
    } else{
include('modules/front_page.php'); //nothing selected, the module "front_page" is included
    }
?>

 

 

####the first type of view- a section has just  a single page under it

####the page.php is included if "index.php?sec=1&view=page" was selected

page.php

<?php
if(isset($_GET['pag'])){
    $page_set = get_page_by_id($_GET['pag']);
}else{
    $page_set = get_pages_by_section_id($_GET['sec']);
}
    $page = mysql_fetch_array($page_set);
    if ($page['extra_module'] != NULL){
        $module = "modules/" . $page['extra_module'] . ".php";
        include $module;
    } else{
        echo "<h2>" . $page['name'] . "</h2><br />";
        echo $page['content'];
    }
?>

 

####second type of view - a section has more pages under it but I want to see a list of them with the link to the full article for each link

####index.php?sec=2&view=pages_list

####if I choose any of the links then selected_item_id() will give me the "pag" and then the pag.php will be included

 

pages_list.php

<ul class="pags">
<?php
$page_set = get_pages_by_section_id($_GET['sec']);
while($page = mysql_fetch_array($page_set)){
    echo "<li><a href=\"index.php?pag=" . urldecode($page['id']) . "\"> {$page['name']}</a></li>";
}
?>
</ul>

 

####the third type of view - a section has more categories, and no direct articles under it. It will give a list of all the categories

####index.php?sec=3&view=categories_list

if I choose any of the links then selected_item_id() will give me the "cat"

 

categories_list.php

<ul class="cats">
    <?php
    $category_set = get_categories_by_section_id($_GET['sec']);
    while($category = mysql_fetch_array($category_set)){
        echo "<li><a href=\"index.php?cat=" . urldecode($category['id']) . "\"> {$category['name']}</a></li>";
    }
    ?>
</ul>

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.