I am not sure whether there is a ‘right’ or ‘wrong’ answer to this one, but I was curious about the general consensus.
I have a User model that currently performs user functions (such as retrieving user details from the database, inserting new users to the database, updating user details etc.)
This leads to a fairly large class.
Is there any accepted thought on the breaking up of models, when the model already relates to one kind of entity? Or are large classes permissible in this case?
At the moment, the class will resemble something like this:
<?php
class Users_Model
{
public function userExists($user_id)
{
//Code to check if user exists
}
public function fetchUser($user_id)
{
//code to fetch User object from ID...
}
protected function fetchUserPassword($user_id)
{
return $this->fetchUserValue($user_id, "password");
}
protected function fetchUserLowercaseUsername($user_id)
{
return $this->fetchUserValue($user_id, "lower");
}
private function fetchUserValue($user_id, $value)
{
//fetch a user value from the database
}
public function logout()
{
//code to log user out
}
public function doesPasswordMatch($user_id, $password)
{
//checks if inputted password matches that which is on record...
}
public function createSession($user_id)
{
//creates sessions for DATABASE
}
private function createCookie($code, $user_id)
{
//create cookies for session
}
}
?>
My concern was doing too many things (it was performing retrieval of user information from the database, and handling the database side of logging out and sessions for the user).
4
Permissible?…yes
I would break-up a User class into parts in cases where it might need to scale really big.
Model Breakdown Example (Composition):
User HAS
LoginCredentialsContactInfoFavoritesFriendsRoles
Ask yourself whether the class has more than one reason to change.
Acording to Bob Martin’s explanation of Single Responsability Principle:
As an example, consider a module that compiles and prints a report.
Such a module can be changed for two reasons. First, the content of
the report can change. Second, the format of the report can change.
These two things change for very different causes; one substantive,
and one cosmetic. The single responsibility principle says that these
two aspects of the problem are really two separate responsibilities,
and should therefore be in separate classes or modules. It would be a
bad design to couple two things that change for different reasons at
different times.
The example talks about separation of cosmetics from substantial, but it applies to any other two or more concerns, like business logic, persistence, logging, formatting, etc.
If you think your class ( model or not ) has more than one reason to change, then split it.
It all comes down to separation of concerns and the single responsibility principle (SRP). Of course the question is what that looks like in practice, which is what you are asking. Having worked with MVC for many years these would be my suggestions:
1) I normally consider SRP, as applied to models, to mean that they are responsible for fetching and saving data. This also means that, in practice, they become responsible for data cleaning and validation because both of those things have to happen before any data is saved. So if your model worked with one table (the user’s table) and both retrieved and saved data, then I would say that you are on the right track. So those first 5 functions are perfectly reasonable from a SRP perspective.
2) I would consider logging out/logging in to be a separate responsibility. User authentication has its own complicated set of rules: users may be deactivated, their account may not be verified, their membership may have expired, they may be trying to access a restricted part of the site, etc. The result is that user authentication is its own can of worms and deserves its own class. In my systems I have a user object responsible for fetching/updating user data, but then I have a “logged” class responsible for everything I just mentioned. Not surprisingly, the logged class uses the user class to fetch the user data, and then has its own set of functionality to determine if a particular login is valid.
3) In the case I’ve described, the logged and user class inevitably end up closely related and often rely on eachother. So some care is needed to make sure they don’t end up tied too closely together and to avoid duplication of code. As a general rule of thumb I let the user class stick to data retrieval. So the logged class might use the user class to fetch the user’s password or their assigned roles, but then it will determine if an incoming password is valid, or if the user is allowed to login given the current data from the user object. The logged class may or may not be responsible for persisting the users session: that depends on your architecture, although ideally the logged class wouldn’t interact with the session persistence mechanism directly. Instead it would operate through a general purpose session class.
4) To some extent properly executing SRP is more of an art than a science. You know it is working well when you either a) get input from someone else who has been doing it for a while or b) you use it yourself for a long while and put it through its paces. To some extent you know whether or not your code is working by how much it gets in your way as you actually use it.