HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Determining what functions should be implemented in this user-authentication class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisdeterminingwhatuserimplementedauthenticationshouldfunctionsclass

Problem

Long story short, I would like to use OOP for my new PHP project. It has a "login" requirement (i.e. user will first need to enter the username and password (login.php), if it's correct, it will be redirected to index.php and then fetch some products information). Also there will be a settings page that allows user to change the password, and email if necessary.

In summary:

  • When user attempts to login, the program will check if the username, password entered is correct or not. If it's correct, username (or maybe the whole User object) will be saved in the $_SESSION.



  • When the user settings page is loaded, it will display all user related fields on screen (i.e. by using $_SESSION).



  • The user can make changes in the user settings pages and data should be saved to the database.



I have this User class:

class User {
     private $username;
     private $email;
     private $hashed_password;
     private $dob;
...

     public __construct($user) {
          $this->username = $user;
     }

     public function isPasswordCorrect($userenteredpassword) {
           $this->loadFromDatabase();
           //hashed userenteredpassword, compare it against $this->hashed_password
           //if it matches, return true, otherwise return false.
     } 

     public function saveToDatabase() {
          //save info to database
     }

     public function loadFromDatabase() {
         $row = select fields from user table where username = $this->username limit 1
         if ($row is found) {
              $this->hashed_password = $row['hashed_password'];
              $this->dob = $row['dob'];
              ...
         }
     }
}//class


However, I was told that the function isPasswordCorrect shouldn't be implemented inside class User, as it is not its job to determine whether the login is valid or not. Instead, I should create an authService helper and use it to determine whether the login is good or not.

Now, this makes me wonder if the rest of my

Solution

The Single Responsibility Principle is a good thing to aim for, for many classes. It doesn't always mean that each class only does one thing (though it can), but that it is responsible for a small amount of tightly-related functionality.

That said, the job of some classes is to collect together a number of responsibilities. User may well be one of these, but the individual responsibilities can still be delegated to other classes - e.g. you could always give User an instance of a Credentials class that deals with the password checking.

You almost certainly don't want your database-related functions in User as if you write any tests for User, many of them are likely to have a need to hit the database when they run.

Context

StackExchange Code Review Q#31015, answer score: 2

Revisions (0)

No revisions yet.