CODEWITHSUNDEEP
phpoop
Qiao
Qiao

Reputation: 17049

How to Initialize object by different properties?

There is an object, which can be initialized by id or by name.

How it should be handled?

class user
{
    function __construct($id_or_name)
    {
        if ( is_numeric($id_or_name) )
        {
            $this->id = $id_or_name;
            $this->populate_by_id($this->id)
        }
        else
        {
            $this->name = $id_or_name;
            $this->populate_by_name($this->name)
        }
    }
    ...
}

$user1 = new user('Max');
$user2 = new user(123);

Is it ok for general practice?

Upvotes: 1

Views: 121

Answers (6)

Gordon
Gordon

Reputation: 316989

What you describe could roughly be an ActiveRecord, but ActiveRecord has a couple of drawbacks and isnt a too clean pattern. If you dont know the pattern and its implications, dont use it. Have a look at DataMapper or Repository patterns instead.

Here is an easy example implementation:

class Users
{
    protected $users = array();
    protected $userTableDataGateway;
    protected $userFactory;

    public function __construct($userTableDataGateway, $userFactory)
    {
       $this->userTableDataGateway = $userTableDataGateway;
       $this->userFactory = $userFactory;
    }

    public function findById($id)
    {
        if(false === isset($this->users[$id])) {
            $userData = $this->userTableDataGateway->findById($id);
            $this->users[$id] = $this->userFactory->createById($id, $userData);
        }
        return $this->users[$id];
    }
    …
}

Now instead of doing things like

$jane = new User(123);
$john = new User('john');

and having the User class fetch the data, you create the Users collection once in your bootstrap:

$users = new Users(
    new UserTableDataGateway(
        new DbAdapter('localhost', 'user', 'pass', 'tableName');
    ),
    new UserFactory
);

and whenever you need to load a user from your database you simply do

$jane = $users->findById(123);
$john = $users->findByName('john');

This has the added benefit that your classes are much more dedicated in their responsibilites. This will lead to more maintainable, testable and reusable code. See SOLID for details.

As for your specific piece of code: avoid doing work in the constructor. Dont mix Business Logic with Query or Creation Logic (unless it's an ActiveRecord). Dont combine arguments. Use two arguments or make two methods instead.

Upvotes: 0

J0HN
J0HN

Reputation: 26941

IMHO, it's terrible. Introuce two static "fabric methods", one receive string, other - integer. And construct your object differently:

class user
{
    //you might want make it private
    function __construct($id_or_name)
    {
        //common logic
    }

    static function construct_by_name($name){
        $result = new self();
        $result->name = $name; //note you are in the same class, so you may directly assign private variables
        $result ->populate_by_name($result ->name);
        return $result;
    }

    static function construct_by_id($id){
        $result = new self();
        $result->id= $id; //note you are in the same class, so you may directly assign private variables
        $result ->populate_by_id($result ->id);
        return $result;
    }
}

Upvotes: 2

Wesley van Opdorp
Wesley van Opdorp

Reputation: 14941

I expect the part of the code to know wether to load by name or id, and not let my constructor handle this.

$oUser = new User();
$oUser->loadByName($sName);


$oAnotherUser = new User();
$oAnotherUser->loadById($iId);

Upvotes: 0

CodeCaster
CodeCaster

Reputation: 151594

So a user can't be named 123? Do you catch that in your registration form? ;-) Since it's not clear, as an objective reader of that code, to see what the constructor does, I would change it.

Why don't you use a basic constructor which does less, and call the appropriate method (either retreive_by_id() or retreive_by_name() to retreive the user?

Upvotes: 0

Rijk
Rijk

Reputation: 11301

It's OK, but I'd say this makes for a more clearer interface:

class user {
    public function __construct( $id ) {
        $this->load( $id );
    }

    public function load( $id ) {
        $this->populate_by_id( $id )
    }

    public function loadByName( $name ) {
        $this->populate_by_name( $name )
    }
}

$user1 = new user();
$user1->loadByName('Max');
$user2 = new user(123);

With this approach, you also have don't have the (theoratical) downside that you can't have numeric usernames.

Upvotes: 0

Raouf Athar
Raouf Athar

Reputation: 1813

I think that is fine. There is nothing wrong about how you have accomplished this. You can avoid using an extra variable and write the following:

$this->populate_by_id($id_or_name)

instead of the following two lines:

$this->id = $id_or_name;
$this->populate_by_id($this->id)

Upvotes: 0

Related Questions