Returning a value in constructor function of a class

PhpOopClassConstructor

Php Problem Overview


So far I have a PHP class with the constructor

public function __construct ($identifier = NULL)
{
 // Return me.
if ( $identifier != NULL )
{
  $this->emailAddress = $identifier;
  if ($this->loadUser() )
    return $this;      
  else
  {
// registered user requested , but not found ! 
return false;
  }
}

the functionality of loadUser is to look up the database for a particular email address. When i set the identifier to some email that i'm sure it's not in the database; the first IF is get passed, and goes to the first ELSE. here the constructor should return FALSE; but instead, it returns an object of the class with all NULL values !

how do i prevent this? thanks

EDIT:

thank you all for the answers. that was quite fast ! I see that the OOP way is to throw an Exception. So a throw one, my question changes that what should i do with the exception?? php.net's manual is pretty confusing !

	// Setup the user ( we assume he is a user first. referees, admins are   considered users too )
	try { $him = new user ($_emailAddress);
	} catch (Exception $e_u) { 
	  // try the groups database
	  try { $him = new group ($_emailAddress); 
	  } catch (Exception $e_g) {
	      // email address was not in any of them !!  
	    }
	}

Php Solutions


Solution 1 - Php

Constructors don't get return values; they serve entirely to instantiate the class.

Without restructuring what you are already doing, you may consider using an exception here.

public function __construct ($identifier = NULL)
{
  $this->emailAddress = $identifier;
  $this->loadUser();
}

private function loadUser ()
{
    // try to load the user
    if (/* not able to load user */) {
        throw new Exception('Unable to load user using identifier: ' . $this->identifier);
    }
}

Now, you can create a new user in this fashion.

try {
    $user = new User('[email protected]');
} catch (Exception $e) {
    // unable to create the user using that id, handle the exception
}

Solution 2 - Php

The best you can do is what Steve has suggested. Never create constructors that do any job other then assigning constructor parameters to the object properties, maybe create some default ones, but nothing else. Constructors are meant to create a fully functional object. Such an object must always work as expected after its instantiation. A user has email, name and probably some other properties. When you want to instantiate a user object, give all those properties to its constructor. Throwing exceptions is not a good way either. An exception is meant to be thrown under exceptional conditions. Asking for a user by email is nothing exceptional, even if you eventualy figure out that no such user exists. Exception could be for example if you ask for a user by email = '' (unless that is a regular state in your system, but id rather suggest emails to be null in those cases). To get all those properties for a user object you should have a factory (or a repository if you prefer) object (yes, an object - it is a bad practice to use static whatever) Private constructor is a bad practice either (you'll need a static method anyway and as i already stated, statics are very bad)

so the result should be something like this:

class User {
  private $name;
  private $email;
  private $otherprop;

  public function __construct($name, $email, $otherprop = null) {
    $this->name = $name;
    $this->email = $email;
    $this->otherprop = $otherprop;
  }
}

class UserRepository {
  private $db;

  public function __construct($db) {
    $this->db = $db; //this is what constructors should only do
  }

  public function getUserByEmail($email) {
    $sql = "SELECT * FROM users WHERE email = $email"; //do some quoting here
    $data = $this->db->fetchOneRow($sql); //supose email is unique in the db
    if($data) {
      return new User($data['name'], $data['email'], $data['otherprop']);
    } else {
      return null;
    }
  }
}

$repository = new UserRepository($database); //suppose we have users stored in db
$user = $repository->getUserByEmail('[email protected]');
if($user === null) {
  //show error or whatever you want to do in that case
} else {
  //do the job with user object
}

See? no statics, no exception, simple constructors and very readable, testable and modificable

Solution 3 - Php

The constructor is suppose to create an object. Since in php booleans are not considered to be objects the only option is null. Otherwise use a workaround i.e. write a static method which creates the actual object.

public static function CheckAndCreate($identifier){
  $result = self::loadUser();
  if($result === true){
    return new EmailClassNameHere();
  }else{
    return false;
  }
}

Solution 4 - Php

I'm really surprised that for 4 years none of the 22k viewers suggested creating private constructor and a method that attempts to create an object like this:

class A {
	private function __construct () {
		echo "Created!\n";
	}
	public static function attemptToCreate ($should_it_succeed) {
		if ($should_it_succeed) {
			return new A();
		}
		return false;
	}
}

var_dump(A::attemptToCreate(0)); // bool(false)
var_dump(A::attemptToCreate(1)); // object(A)#1 (0) {}
//! new A(); - gives error

This way you get either an object or false (you can also make it return null). Catching both situations is now very easy:

$user = User::attemptToCreate('[email protected]');
if(!$user) { // or if(is_null($user)) in case you return null instead of false
    echo "Not logged.";
} else {
    echo $user->name; // e.g.
}

You can test it right here: http://ideone.com/TDqSyi

I find my solution more convenient to use than throwing and catching exceptions.

Solution 5 - Php

A constructor cannot return anything but the object it is attempting to create. If the instantiation doesn't complete properly, you'll be left with a class instance full of NULL properties as you've discovered.

If the object loads in an incomplete or error state, I would suggest setting a property to indicate that.

// error status property
public $error = NULL;

public function __construct ($identifier = NULL)
{
 // Return me.
if ( $identifier != NULL )
{
  $this->emailAddress = $identifier;
  if (!$this->loadUser() )
  {
   // registered user requested , but not found ! 
   $this->error = "user not found";
  }
}

When instantiating the object then, you can check if it has an error status:

$obj = new MyObject($identifier);
if (!empty($obj->error)) {
   // something failed.
}

Another (perhaps better) alternative is to throw an exception in the constructor, and wrap the instantiation in a try/catch.

Solution 6 - Php

Why not simply pass the results into the constructor needed to build the object, rather than try to make the constructor fail sometimes?

Even if you could make it fail sometimes, you will still need to check after calling the constructor to ensure it actually did construct, and in those lines, you could just call the ->loadUser() and pass the results into the constructor.

A good hint someone one told me, "always give the constructor what it needs to build the object, don't make it go looking for it."

public function __construct ($emailInTheDatabase, $otherFieldNeeded)
{
    $this->emailAddress = $emailInTheDatabase;
    $this->otherField = $otherFieldNeeded;
}

Solution 7 - Php

thanks for all the comments and solutions. here is what i done to fix the problem: (i hope it helps others)

// Setup the user ( we assume he is a user first. referees, admins are considered users too )
	try {
	  $him = new user ($_emailAddress); 
	  // check the supplied password 
	  $pass_ok = $him->auth($_Password);
	  
	  // check the activation status 
	  $active_ok = $him->makeActive();
	  
	} catch (Exception $e_u) { 
	  // try the groups database
	  try { 
	  $him = new group ($_emailAddress);
	  // check the supplied password 
	  $pass_ok = $him->auth($_Password);
              //var_dump ($pass_ok);
	  
	  // check the activation status 
	  $active_ok = $him->makeActive();
	  } catch (Exception $e_g) {
	      // email address was not in any of them !!
	      $pass_ok = false; $active_ok = false;
	    }
	}

Solution 8 - Php

I wouldn't put too much in the construct. You should consider a static functin that create the User (factory) instead of putting everything in the constructor. Thus, you can still use your user object without having to call implicitly the load function. This will save you pain.

public function __construct(){}

public function setIdentifier($value){
    $this->identifier = $value;
}

public function load(){
    // whatever you need to load here
    //...
    throw new UserParameterNotSetException('identifier not set');
    // ...
    // if user cannot be loaded properly
    throw new UserNotFoundException('could not found user');
}

public static function loadUser($identifier){
    $user = new User();
    $user->setIdentifier($identifier);
    $user->load();
    return $user;
}

Sample usage:

$user = new User(); 
try{
    $user->setIdentifier('identifier');
    $user->load();
}
catch(UserParameterNotSetException $e){
    //...
}
catch(UserNotFoundException $e){
    // do whatever you need to do when user is not found
}

// With the factory static function:
try{
    $user2 = User::loadUser('identifier');
}
catch(UserParameterNotSetException $e){
    //...
}
catch(UserNotFoundException $e){
    // do whatever you need to do when user is not found
}

Attributions

All content for this solution is sourced from the original question on Stackoverflow.

The content on this page is licensed under the Attribution-ShareAlike 4.0 International (CC BY-SA 4.0) license.

Content TypeOriginal AuthorOriginal Content on Stackoverflow
QuestionAnoosh RavanView Question on Stackoverflow
Solution 1 - PhperiscoView Answer on Stackoverflow
Solution 2 - PhpslepicView Answer on Stackoverflow
Solution 3 - PhpworengaView Answer on Stackoverflow
Solution 4 - PhpAl.G.View Answer on Stackoverflow
Solution 5 - PhpMichael BerkowskiView Answer on Stackoverflow
Solution 6 - PhpSteveView Answer on Stackoverflow
Solution 7 - PhpAnoosh RavanView Answer on Stackoverflow
Solution 8 - PhpNicoView Answer on Stackoverflow