patternphpMinor
Determining what functions should be implemented in this user-authentication class
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:
I have this
However, I was told that the function
Now, this makes me wonder if the rest of my
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
Userobject) 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'];
...
}
}
}//classHowever, 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.
You almost certainly don't want your database-related functions in
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.