patternphpMinor
PHP, would this class pass the single responsibility principle?
Viewed 0 times
thisprinciplethepassphpwouldsingleresponsibilityclass
Problem
I have a user class comprised of the code below. It has functions to insert a user, update a user, delete a user, look up a user, and search for users. What I'm wondering is if I this class would pass the Single Responsibility Principle as stated here (http://en.wikipedia.org/wiki/Single_responsibility_principle).
```
class user{
$first_name = "";
$last_name = "";
$physical_address1 = "";
$physical_address2 = "";
$physical_city = "";
$physical_state = "";
$physical_zip = "";
$mailing_address1 = "";
$mailing_address2 = "";
$mailing_city = "";
$mailing_state = "";
$mailing_zip = "";
function __construct($array){
if(!isset($array['first_name']) && !empty($array['first_name'])){
$this->first_name = "";
}
if(!isset($array['last_name']) && !empty($array['last_name'])){
$this->last_name = "";
}
if(!isset($array['physical_address1']) && !empty($array['physical_address1'])){
$this->set_physicalAddress(array(
"address_1"=>$array['physical_address1'],
"address_2=>$array['physical_address2']",
"city"=>$array['physical_city'],
"state"=>$array['physical_state'],
"zip"=>$array['physical_zip']
));
}
if(!isset($array['mailing_address1']) && !empty($array['mailing_address1'])){
$this->set_mailingAddress(array(
"address_1"=>$array['mailing_address1'],
"address_2=>$array['mailing_address2']",
"city"=>$array['mailing_city'],
"state"=>$array['mailing_state'],
"zip"=>$array['mailing_zip']
));
}
}
function set_physicalAddress($array){}
function get_physicalAddress(){}
function set_mailingAddress($array){}
function get_mailingAddress(){}
static function insert($array){}
static function update($array){}
static
```
class user{
$first_name = "";
$last_name = "";
$physical_address1 = "";
$physical_address2 = "";
$physical_city = "";
$physical_state = "";
$physical_zip = "";
$mailing_address1 = "";
$mailing_address2 = "";
$mailing_city = "";
$mailing_state = "";
$mailing_zip = "";
function __construct($array){
if(!isset($array['first_name']) && !empty($array['first_name'])){
$this->first_name = "";
}
if(!isset($array['last_name']) && !empty($array['last_name'])){
$this->last_name = "";
}
if(!isset($array['physical_address1']) && !empty($array['physical_address1'])){
$this->set_physicalAddress(array(
"address_1"=>$array['physical_address1'],
"address_2=>$array['physical_address2']",
"city"=>$array['physical_city'],
"state"=>$array['physical_state'],
"zip"=>$array['physical_zip']
));
}
if(!isset($array['mailing_address1']) && !empty($array['mailing_address1'])){
$this->set_mailingAddress(array(
"address_1"=>$array['mailing_address1'],
"address_2=>$array['mailing_address2']",
"city"=>$array['mailing_city'],
"state"=>$array['mailing_state'],
"zip"=>$array['mailing_zip']
));
}
}
function set_physicalAddress($array){}
function get_physicalAddress(){}
function set_mailingAddress($array){}
function get_mailingAddress(){}
static function insert($array){}
static function update($array){}
static
Solution
I don't think the user class you provided satisfies the Single Responsibility Principle.
I can see a few situations that may require changes to the user class.
-
You want to change what information about the user is stored. This could happen if you need to allow a title (Mr., Mrs., etc.) and/or a middle name. Doing so would require a change to the user class.
-
You want to create a new class that also contains address information. The new class would likely duplicate code that is already present in the user class.
-
You want include the user's country with the physical and mailing addresses. Doing so would require a change to the user class. It would also require changes to the class in #2.
-
You want to change where the information is stored. You may decide you want to be able to test your class without needing to connect to the database (assuming you're connecting to a database). Doing so would require a change to the CRUD methods (insert, update, delete, lookup, and search) of your class.
(1) is related to the user class and changing the class is expected. No problem here.
(2), (3), and (4) are really not the responsibility of the user class, yet they would probably require changes to the user class.
You should consider creating an additional "address" class that can be reused for the physical and mailing addresses for the user. The user class could contain instances of the address class, and would not require changes if the address functionality changed. Any new classes that need address information could reuse this class.
You should also consider creating a new class (let's call it userProvider) that is responsible for the CRUD functions. This class could have the CRUD methods with a user object as a parameter. Your user class could still define the CRUD methods if you want, but would simply pass the user instance to the userProvider class, which would be responsible for storing/retrieving the user information.
I can see a few situations that may require changes to the user class.
-
You want to change what information about the user is stored. This could happen if you need to allow a title (Mr., Mrs., etc.) and/or a middle name. Doing so would require a change to the user class.
-
You want to create a new class that also contains address information. The new class would likely duplicate code that is already present in the user class.
-
You want include the user's country with the physical and mailing addresses. Doing so would require a change to the user class. It would also require changes to the class in #2.
-
You want to change where the information is stored. You may decide you want to be able to test your class without needing to connect to the database (assuming you're connecting to a database). Doing so would require a change to the CRUD methods (insert, update, delete, lookup, and search) of your class.
(1) is related to the user class and changing the class is expected. No problem here.
(2), (3), and (4) are really not the responsibility of the user class, yet they would probably require changes to the user class.
You should consider creating an additional "address" class that can be reused for the physical and mailing addresses for the user. The user class could contain instances of the address class, and would not require changes if the address functionality changed. Any new classes that need address information could reuse this class.
You should also consider creating a new class (let's call it userProvider) that is responsible for the CRUD functions. This class could have the CRUD methods with a user object as a parameter. Your user class could still define the CRUD methods if you want, but would simply pass the user instance to the userProvider class, which would be responsible for storing/retrieving the user information.
Context
StackExchange Code Review Q#11809, answer score: 3
Revisions (0)
No revisions yet.