patternphpMinor
A simple User class
Viewed 0 times
usersimpleclass
Problem
// The DataLayer to MySQL
require_once $_SERVER["DOCUMENT_ROOT"] . "/api/DAL/classes/User.dal.php";
class User
{
public $id;
public $name;
public $age;
public function __construct($id = null)
{
$this->dl = new UserDAL();
// Get an existing user object
if (!empty($id)) {
$user = $this->getUsers($id);
$this->id = $user->id;
$this->name = $user->name;
$this->age = $user->age;
}
}
public function create($user)
{
// Create new user
$this->name = $user->name;
$this->age = $user->age;
return $this->dl->create($user);
}
public function update($user)
{
// Update existing user
$this->id = $user->id;
$this->name = $user->name;
$this->age = $user->age;
return $this->dl->update($user);
}
public function delete($user)
{
// Create new user
$this->id = $user->id;
return $this->dl->delete($user);
}
public function getUsers($id)
{
return $this->dl->getUsers($id);
}
}
// Some tests...
// Get existing user
$existing_user = new User("1");
var_dump($existing_user);
// Create a new user
$new_user = new User();
$new_user->age = 23;
$new_user->name = "Bob";
$new_user->create($new_user);
var_dump($new_user);
// Update existing user
$update_user = new User();
$update_user->id = "1";
$update_user->name = "Henry";
$update_user->age = 100;
$update_user->update($update_user);
var_dump($update_user);
// Delete a user
$delete_user = new User();
$delete_user->delete("1");Solution
The comment by mferly is very constructive:
Comment your code/methods/properties/everything. Seriously. In your
The source of the
I'd also suggest considering following the singleton pattern with respect to how the
Even if instances of that class are not very demanding of resources it may still be beneficial to reduce the number of instances as much as possible for other reasons. Does each instance of the DAL create a connection to the database? If so, that may be an important reason to minimize the number of instances.
Comment your code/methods/properties/everything. Seriously. In your
getUsers() method, is the $id argument/value an array or integer I know it's not an array)? I ask because getUsers() implies that you're returning multiple users, not just one. Otherwise it'd be named getUser() See the discrepancy and potential confusion?The source of the
getUsers() method from the class stored at $_SERVER["DOCUMENT_ROOT"] . "/api/DAL/classes/User.dal.php" was not included so we can only guess as to how it functions.I'd also suggest considering following the singleton pattern with respect to how the
dl property is created in the constructor for each instance:$this->dl = new UserDAL();Even if instances of that class are not very demanding of resources it may still be beneficial to reduce the number of instances as much as possible for other reasons. Does each instance of the DAL create a connection to the database? If so, that may be an important reason to minimize the number of instances.
Code Snippets
$this->dl = new UserDAL();Context
StackExchange Code Review Q#130061, answer score: 3
Revisions (0)
No revisions yet.