3

I am working on an extremely old project that has migrated from PHP 4 to PHP 5.2 and we are now migrating the project to PHP 5.4. We've come up with a number of reference errors. Here is some example code that's bugging me.

<?php

class book_shelf 
{
    protected $_elements = array();

    function addBook(&$element){
        $this->_elements[] =& $element;
    }

    function printBooks(){
        print(json_encode($this->_elements));
    }
}

class book 
{
    function __construct($title, $author="unknown") {
        $this->title = $title;
        $this->author = $author;
    }

    public $title = NULL;
    public $author = NULL;
}

function createBookShelf(){
    $bookshelf1 = new book_shelf();

    for ($i = 0; $i < 3; $i++){
        $book1 = new book("Book $i");
        $bookshelf1->addBook($book1);
    }

    $bookshelf1->printBooks();  
}   

createBookShelf();

?>

I would expect this to create 3 separate books. But instead every object in the elements array ends up pointing to the last variable. On the other hand if I create a new book by reference everything works right. (e.g. $book1 =& new book("Book $i");) Here is the example code:

<?php

class book_shelf 
{
    protected $_elements = array();

    function addBook(&$element) {
        $this->_elements[] =& $element;
    }

    function printBooks(){
        print(json_encode($this->_elements));
    }
}

class book 
{
    function __construct($title, $author="unknown") {
        $this->title = $title;
        $this->author = $author;
    }

    public $title = NULL;
    public $author = NULL;
}

function createBookShelf() {
    $bookshelf1 = new book_shelf();

    for ($i = 0; $i < 3; $i++){
        $book1 =& new book("Book $i");
        $bookshelf1->addBook($book1);
    }

    $bookshelf1->printBooks();  
}   

createBookShelf();

I was under the impression that creating an object by reference in PHP 5.4 was not necessary. Any idea why I am getting different results?

4
  • 3
    Remove all references to objects. Objects are handled (assigned, passed to functions/methods, returned) by reference by default. Feb 8, 2013 at 23:34
  • $this->_elements[] =& $element; What did you expect this to do? Feb 8, 2013 at 23:35
  • Removing the & from this line $this->_elements[] =& $element; fixes the issue Feb 8, 2013 at 23:45
  • 1
    it is not completely true that all objects are handled by reference since PHP 5. look here
    – bitWorking
    Feb 8, 2013 at 23:47

2 Answers 2

2

This is a logical error and not PHP version related. Lets have a look at the following snippets of your code:

//echo "phpinfo: " . phpinfo();
for ($i = 0; $i < 3; $i++){
    $book1 = new book("Book $i");
    $bookshelf1->addBook($book1);
}

...

function addBook(&$element){
    $this->_elements[] =& $element;
}

What is happening? You passing $book by reference. But in every loop you change the value of the reference. At the end of the for loop $book points to the last created book object and so the references. (had this error once too ;)

Conclusion: Your addBook function has to look like this:

function addBook($element){
    $this->_elements[] = $element;
}

and keep the code that adds the book unchanged - meaning like in the above example without the &.


*About =& new ... *

First, if you doing this you'll get the following warning as of php 5.3 :

Deprecated: Assigning the return value of new by reference is deprecated

Therefore you should not use it for new code.

3
  • Can you explain why $book1 =& new book("Book $i"); functions differently? The docs seemed to suggest that "... =& new ..." now functions the same as "... = new ..." though that clearly isn't the case. My assumption is that even though PHP now uses a reference by default when assigning during instantiation, the "breaking" of the previous reference that =& provided (as explained here stackoverflow.com/a/3200017/459966) is still in force.
    – Aaronius
    Feb 8, 2013 at 23:57
  • While I'm sure Brad appreciates everyone's comments thus far, I don't think anyone has actually answered his question, that is: "Any idea why I am getting different results?" It appears he already knows how to fix it but wants to know why his two examples behave differently.
    – Aaronius
    Feb 9, 2013 at 0:27
  • @Aaronius I've could simplify the problem. Can you explain this: pastebin.com/Sq0HFxXv ?
    – hek2mgl
    Feb 9, 2013 at 2:03
1

As you write this is PHP 4 code originally. These two constructs:

$book1 =& new book("Book $i");

and

function addBook(&$element){
    $this->_elements[] =& $element;
}

are not needed any longer. The first one is "new by reference" which never was correct to do but necessary to get something working in PHP 4 with objects and not cloning objects all the time

The second is "pass by reference" which can be a valid concept, but this is not necessary as far as objects are concerned in PHP 5 and should not be used for parameters that are objects.

The memory managements has been greatly improved in PHP 5 and then again in PHP 5.3. PHP 5.4 helps you to migrate the code by giving warnings. Especially "new by reference" can be easily spotted and fixed.

Old (PHP 4):

$book1 =& new book("Book $i");

New (Current PHP versions):

$book1 = new book("Book $i");

(and no, it will not come back :)). For the methods where you pass by reference I suggest when you refactor the code, remove the references and and also type-hint the object type:

Old (PHP 4):

function addBook(&$element){
    $this->_elements[] =& $element;
}

New (current PHP versions):

function addBook(Book $element){
    $this->_elements[] = $element;
}

Then you not only fix an old problem but you also ensure that this function is only invoked with the intended type and you can see it was always meant to take an object (and not a variable reference which was only used in PHP 4 for objects because PHP 4 was limited. This is not needed any longer and can (and should, throw away this old cruft!) be removed).

Put your code-base under version control so you can easily apply the changes to your code-base without fearing to destroy anything previously working.


I would expect this to create 3 separate books. But instead every object in the elements array ends up pointing to the last variable.

The actual problem here is that you use the pass by reference. When you assign

$book1 = new book("Book $i");

The refcount of $book1 is one and it's not marked as reference. Passing it now as reference into the $bookshelf1->addBook($book1) method will turn it into a reference with a refcount of 2 because inside that method the reference is assigned again to the private member of the class:

for ($i = 0; $i < 3; $i++)
{
    $book1 = new book("Book $i");
    $bookshelf1->addBook($book1);
}

...

function addBook(&$element){
    $this->_elements[] =& $element;
}

You then change that now created reference.

So now when you change the code with the instantiation to:

$book1 = & new book("Book $i");

You always - once per loop - create a new alias (reference) only with the same variable name. Because you then pass this as reference, it is a new alias you assign as reference. Therefore the array inside Book points to three different references - not three times to the same reference.

I hope this explains the issue better. Also this is the reason why we say: Prevent using references unless you know what you do. The PHP 4 situation was that many users used to use references without fully understanding how that works (which is understandable because it's not easy in PHP, I also need to wrap my head around it a bit until I find a good description to share here - hopefully it worked this time :)). The benefit of using PHP 5 here clearly is that you don't need to use references any longer for objects which makes it easier to write and read the code.


Related:

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.