Pass object into method, assign that object to another object, passed in object still the same object I passed in

Question

I implemented a little IdentityMap into my DataMappers and it works correctly in the way that it knows if an object has already been loaded but it does not assign the in memory object properly.

I have simplified the code down as much as I can (it's not complicated anyway) to one entity, no database etc. Could someone please explain why in the lookup() method does not properly assign the already loaded Customer object to the passed in Customer object?

Customer.php

class Customer {

    private $id;
    private $name;

    public function getId() {
        return $this->id;
    }
    public function setId($id) {
        $this->id = $id;
    }

    public function getName() {
        return $this->name;
    }
    public function setName($name) {
        $this->name = $name;
    }

}

CustomerMapper

class CustomerMapper {

    private $identityMap;

    public function __construct(IdentityMap $identityMap) {
        $this->identityMap = $identityMap;
    }

    public function fetch(Customer $customer) {

        if( $this->identityMap->lookup($customer) ) {
            return true;
        }

        $this->assign($customer, array('id' => 1, 'name' => 'John'));   
    }

    private function assign(Customer $customer, Array $row) {

        $customer->setId($row['id']);
        $customer->setName($row['name']);

        $this->identityMap->add($customer); 
    }

}

IdentityMap

class IdentityMap {

    private $customers;

    public function lookup(Customer $customer) {

        if( !array_key_exists($customer->getId(), $this->customers) ) {
            return false;
        }

        $customer = $this->customers[$customer->getId()]; //Something wrong here?

        return true;
    }

    public function add(Customer $customer) {
        $this->customers[$customer->getId()] = $customer;
    }

}

When I then run this:

$identityMap = new IdentityMap();
$customerMapper = new CustomerMapper($identityMap);

for( $i = 0; $i < 3; $i++ ){

    $customer = new Customer();
    $customer->setId(1);

    $customerMapper->fetch($customer);

    echo 'ID: ' . $customer->getId() . '<br>Name: ' . $customer->getName() . '<br><br>';

}

Output:

ID: 1
Name: John

ID: 1
Name:

ID: 1
Name:

Why do the second and third Customer objects have no name? I am fairly sure there is a problem in the assigning part in the lookup() method. I have been at this since last night trying and reading everything.

I have changed the lookup() methods signature to having the "&" symbol in front of the passed in object with no luck.

Try this, public function lookup(Customer &$customer) so that it's passed by reference and not by value. — Mike Perrenoud
– Mike Perrenoud, Commented Jun 3, 2013 at 12:34
@MichaelPerrenoud tried it and no luck. Any other suggestions? It should work in theory though shouldn't it? I don't know what's wrong. — ibanore
– ibanore, Commented Jun 3, 2013 at 12:47

Bere · Accepted Answer · 2013-06-03 14:06:54Z

The problem is

When fetch() is called In the first loop and it in turn calls lookup() it won't find any value (since identityMap is empty) as a result $customer will be given new value in assign() (in this case $customer->name = 'John' and $customer->id='1'). Here note that id is not provided by $customer->setId(1);. Whatever value you give $this->assign() modifies the original id value of $customer(passed by reference) by assigning id value to 1. You can test it by changing 1 to any arbitrary value (BTW if you change 1 to 3 for example, you will see all the results).

So in the first loop $customer is populated with all the values to display properly (id->1 and name->'john')

But in the second loop

if( $this->identityMap->lookup($customer) ) {
    return true;
}

returns true. (a customer object with id =1 is found in $identityMap; so it is not modifying the $customer object passed as an argument.) which means, the function returns before name value is assigned to $customer.

So starting from the second loop

for( $i = 0; $i < 3; $i++ ){
...
$customer->setId(1);
...
}

the newly created $customer objects will not be assigned name value. That's why it is displayed with id values only.

You can solve the above problem by applying the following changes:

function lookup(){
...
return $customer; // instead of returning true
}

function fetch(){
...
$c=$this->identityMap->lookup($customer);//
if($c){
    $customer->name=$c->getName();
}

// if you like the new objects hold their original value do the following
$this->assign($customer, array('id' => $customer->getId(), 'name' => 'John'));

Thanks for the reply. I have now made my fetch() method return the object instead of just returning true or false.

Markus Schindler · Accepted Answer · 2013-06-03 12:46:48Z

1

You are adding 3 customers to the lookup with the same key (id) after the first for loop run the fetch method returns true for the rest of the for loop runs. so the name will never be set.

You could try this:

    if( $this->identityMap->lookup($customer) ) {
        return $this->identityMap->get($customer);
    }

But dont forget to implement the method "get" in the IdentityMap Class ;)

edited Jun 3, 2013 at 12:46

answered Jun 3, 2013 at 12:38

Markus Schindler

405 bronze badges

6 Comments

ibanore Over a year ago

I'm not because once a customer with ID = 1 has been added the next time the fetch() is run it finds the customer with ID = 1 and returns true.

Bere Over a year ago

Why is it not showing the names? That's part of the question.

Markus Schindler Over a year ago

Because you do not load the data from your lookup. you just return true and all you have is a customer object with only an id set.

Markus Schindler Over a year ago

In your first for loop iteration $this->identityMap->lookup($customer) returns false and you set the name. In the second interation $this->identityMap->lookup($customer) returns true and you DONT set the name

ibanore Over a year ago

That is because if the lookup() method returns true it is supposed to set the passed in Customer object to now point to the already in memory Customer object with ID = 1. But that does not seem to want to work.

|

Collectives™ on Stack Overflow

Pass object into method, assign that object to another object, passed in object still the same object I passed in

2 Answers 2

1 Comment

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related