Should a User class only contain attributes and no methods apart from getters/setters?

Question

I'm trying to improve my OOP code and I think my User class is becoming way too fat.

In my program a user has rights over "lists". Read, Write, Update, Delete. So I made a User class

class User
{
protected $_id;
protected $_email;
protected $_username;
protected $_hashedPassword;

//...Various setters/getters

public function canRead(List $list){
    //Database query verifies if user has READ rights
}

public function canUpdate(List $list){
    //Database query verifies if user has UPDATE rights
}
//etc...
}

• Should canRead, canUpdate, canWrite, canDelete methods be moved to another class (UserAccessCheck or something...)?
• If not, should the actual SQL be moved into the List object (listCanBeReadByUser()) ?

Answer 1

If you have the feeling your classes are too big, you are usually right. Of course there are always different approaches how you could split classes and your idea seems to be reasonable.

So you will have some plain Value Objects (the User), some UserRepository class for getting/updating the data in the database and some kind of UserAccessCheck/PermissionManager/... class for checking the actual user rights.

In case you store your permission also in the database, you can of course extract a PermissionRepository to collect all the SQL statements in one location and don't mix your level of abstractions.

To sum this up, the decision is up to you. A class with 4 properties, some Getters and 4 can...()-methods might be suitable in one scenario and splitting it up into 5 classes might be suitable in an other scenario. In general "good" (tm) developer tend to make there code more general/abstract than necessary and wasting there time or there clients money in premature optimization. "Bad" (tm) developer will never feel the need to split a class. Find the right way in between.