4
\$\begingroup\$

// 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");
\$\endgroup\$
2
  • \$\begingroup\$ Perhaps you would like to post your User.dal.php too, as that would make this a more interesting question. \$\endgroup\$ Jun 3, 2016 at 17:53
  • 2
    \$\begingroup\$ 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? \$\endgroup\$
    – mferly
    Jun 6, 2016 at 18:32

1 Answer 1

3
\$\begingroup\$

The comment by mferly is very constructive:

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.