User class, taking on login and registration capabilities

Question

Based on the Single Responsibility Principle, I have been looking at my User class, trying to figure out how I can break out more functionality into separate classes.

But it seems like all the functions I have belong in the User class.

Such as login and registerUser. I'm not sure it would make sense to create a class just for logging in users. That functionality would belong in the User class, correct?

Same with registerUser. Would it really make sense to create a class just for that action? That wouldn't seem very object oriented to me.

Any ideas on how I could make this abide by the SRP a bit more would be great.

I am used to OOP. And doing things like separating functions from a class to put inside their own class seems a bit strange, and not OOP like.

That is what I have read much of what to do. Separate code into it's own class.

Here is my code, so you can have a look:

class User extends Person
{
    private static $instance;

    private function __construct()
    {
        $this->ID = $_SESSION['userID'];
    }

    public static function getInstance()
    {
        if (!isset(self::$instance)) {
            self::$instance = new User();
        }
        return self::$instance;
    }

    function login($email, $password)
    {
        global $databaseTest;

        $results = $databaseTest->callProc("getUserLogin", array($email), PDO::FETCH_OBJ);
        $data = $results['data'][0];

        if (count((array)$data) >= 1) { //Email exists in database
            if (password_verify($password, $data->password)) {
                if ($data->userActive == "Y") {
                    $_SESSION['userID'] = $data->userid;
                    $_SESSION['email'] = $data->email;
                    $_SESSION['firstname'] = $data->firstname;
                    $_SESSION['lastname'] = $data->lastname;

                    return array('status' => "success");
                } else {
                    return array('status' => "email-notConfirmed");
                }
            } else {
                return array('status' => "password-incorrect");
            }

        } else { //Email doesn't exist in database
            return array('status' => "email-notFound");
        }
    }

    function setActive()
    {
        global $databaseTest;
        $results = $databaseTest->callProc("setUserActive", array($this->ID), PDO::FETCH_OBJ);

        //Return
        $string = ($results['status'] == "success") ? "success" : "failure";
        return array("status" => $string);
    }

    function setInactive()
    {
        global $databaseTest;
        $results = $databaseTest->callProc("setUserInActive", array($this->ID), PDO::FETCH_OBJ);

        //Return
        $string = ($results['status'] == "success") ? "success" : "failure";
        return array("status" => $string);
    }

    function setProfilePic($imgSrc)
    {
        /* Large */
        $path = "c:/webroot/supplyIt/images/users/large/" . $_SESSION['userID'] . ".jpg";
        $status = base64_saveAsJPG($imgSrc, $path, 500, 500);

        if ($status == "success") {
            /* Medium */
            $path = "c:/webroot/supplyIt/images/users/medium/" . $_SESSION['userID'] . ".jpg";
            base64_saveAsJPG($imgSrc, $path, 200, 200);

            /* Small */
            $path = "c:/webroot/supplyIt/images/users/small/" . $_SESSION['userID'] . ".jpg";
            base64_saveAsJPG($imgSrc, $path, 50, 50);

            return array("status" => "success");
        } else {
            return array("status" => "failure");
        }
    }

    function registerUser($firstname, $lastname, $email, $password, $gender)
    {

        if (empty($firstname)) return array('status' => "fn-empty");
        if (strlen($firstname) > 30) return array('status' => "fn-tooLong");
        if (empty($lastname)) return array('status' => "ln-empty");
        if (strlen($lastname) > 30) return array('status' => "ln-tooLong");
        if (empty($email)) return array('status' => "email-empty");
        if (strlen($email) > 100) return array('status' => "email-tooLong");
        if (empty($password)) return array('status' => "password-empty");
        if (strlen($password) < 6) return array('status' => "password-tooShort");
        if (strlen($password) > 50) return array('status' => "password-tooLong");
        if (strpos($email, '@') === false) return array('status' => "email-invalid");
        if (strpos($email, '.') === false) return array('status' => "email-invalid");
        if (empty($gender)) return array('status' => "gender-empty");

        global $databaseTest;

        $results = $databaseTest->callProc("getUser", array($email), PDO::FETCH_OBJ);
        if (count($results['data']) >= 1) return array('status' => "email-alreadyRegistered");

        $hashedPassword = password_hash($password, PASSWORD_DEFAULT);
        $array = array(
            $firstname,
            $lastname,
            $email,
            $hashedPassword,
            $gender
        );
        $results = $databaseTest->callProc("registerInsert", $array, PDO::FETCH_OBJ);

        //Return
        $string = ($results['status'] == "success") ? "success" : "failure";
        return array("status" => $string);
     }
}

Answer 1

We have to separate the register and login implementation into a class and then teach the user model object on how it would implement those function

So in the code you dont directly code in the implementation on how you will save or update a record , you have to inject the class that would do it .

for example in this code

$userPersistor = new RegularUserPersistor();
if (isset($_POST['isPremium'])) {
    $userPersistor = new PremiumUserPersistor();
}
$userObj = new User();
// this line here , instead of writing the code inside registerUser() method , 
// we taught it how to save the record . 
$userObj->setUserRecordPersistor($userPersistor);
$userObj->registerUser();

Now that we implement it this way we can now clearly see Single Responsibility Principle. Checkout class PremiumUserPersistor, it does only one thing and that is to authenticate(User $user) a user no other function is coded there except the authenticate

I would approach it this way. Please read the sample code and if you have question feel free to comment below.

/**
* User model 
*/
class User
{
    //object that would handle the saving of user profile information (fullname , profile picture..etc.)
    protected $userProfilePersistor;
    // object that handles the saving of user information (username , password ...)
    protected $userRecordPersistor;
    // handles the authentication 
    protected $userAuthenticator;

    public function setUserRecordPersistor(UserRecordPersistorInterface $_userRecordPersistor)
    {
        $this->userRecordPersistor = $_userRecordPersistor;
    }
    public function setUserAuthenticator(UserAuthenticatorInterface $_userAuthenticator)
    {
        $this->userAuthenticator = $_userAuthenticator;
    }
    public function registerUser(){
        // in here we will use the userRecordPersistor to save the information
        // with this you can change anytime time you want on how you save the information
        $this->userRecordPersistor->save($this); // or persist()
    }
    // or login if you like
    public function authenticate()
    {
        $isValid = true;
        try {
            $this->userAuthenticator->authenticate($this); // or login()
        } catch (UserPasswordInvalidException $e) {
            // throw it again if you like... or you can return false
            $isValid = false;
        }
        return $isValid;
    }
}
// you may want to put this in separate file but for demonstration purposes lets put it in one file
interface UserAuthenticatorInterface(){
    public function authenticate(User $user);
}
// you may want to put this in separate file but for demonstration purposes lets put it in one file
interface UserRecordPersistorInterface {
    public function save(User $user);
}

/*fun part starts here , */

/**
* Persistor class for regular user . you may want to put this in separate file but for demonstration purposes lets put it in one file
*/
class RegularUserPersistor  implements UserRecordPersistorInterface
{
    public function save(User $user)
    {
        // @TODO - insert the record into the database 
        // @TODO - send normal email confirmation
    }
}
/**
* Persistor class for premium user . you may want to put this in separate file but for demonstration purposes lets put it in one file
*/
class PremiumUserPersistor  implements UserRecordPersistorInterface
{
    public function save(User $user)
    {
        // if user exists in the database you can throw UserExistsException
        // if not , proceed as normal
        // @TODO - insert the record into the database 
        // @TODO - send invoice link , 
        // @TODO - send confirmation code to mobile number 
        // @TODO - other features that you want
    }
}
/* You can do the same with the authenticator . You may want to put this in separate file but for demonstration purposes lets put it in one file */
//create a class that implements UserAuthenticatorInterface interface
class BasicUserAuthenticator implements UserAuthenticatorInterface{
    public function authenticate(User $user)
    {
        /*check database if user exists*/
    }
}
/*You may want to put this in separate file but for demonstration purposes lets put it in one file*/
class PremiumUserAuthenticator implements UserAuthenticatorInterface{
    public function authenticate(User $user)
    {
        /*check database if user exists  */
            /*if user exists send authentication link to his/her email address*/
    }
}



/* lets put into action  , so if you want to register a user ,  */
$userPersistor = new RegularUserPersistor();
if (isset($_POST['isPremium'])) {
    $userPersistor = new PremiumUserPersistor();
}
$userObj = new User();
$userObj->setUserRecordPersistor($userPersistor);
$userObj->registerUser();

/*and for authentication , if you want to authenticate a user be it a regular user or premium user*/
$userAuthenticator = new BasicUserAuthenticator();
if (isset($_POST['isPremium'])) {
    $userAuthenticator = new PremiumUserAuthenticator();
}
$userObj = new User();
$userObj->setUserAuthenticator($userAuthenticator);
$userObj->authenticate();

Answer 2

Separate the register and login functions into a new class called registration and then have the user object as a parameter to those functions.

This is often a criticism of OOP code, when its used to model something that doesn't map on to real world objects then naming can become difficult and increasingly abstract.

Inside registerUser() there are a lot of conditionals that map into status => 'something here' this looks like a data structure to me, maybe in the future you might want to report all the relevant errors in the form instead of returning one?