Creating User objects

Question

I'm creating a PHP object with an array of data:

$row = array(
  'id' => 123,
  'username' => 'Testing',
  'status' => 1,
  'created_on' => '2014-09-20',
  'created_by' => 1,
  'updated_on' => null,
  'updated_by' => null,
);

and I create a corresponding object using the following code:

$model = new User();
$model->id = (int)$row['id'];
$model->username = $row['username'];
$model->status = (int)$row['status'];
if($row['created_on'] != null) {
  $model->created_on = new DateTime($row['created_on']);
}
$model->created_by = (int)$row['created_by'];
if($row['updated_on'] != null) {
  $model->updated_on = new DateTime($row['updated_on']);
}
if($row['updated_by'] != null) {
  $model->updated_by = (int)$row['updated_by'];
}

Is there any way to simplify it?

P.S. I'm not using any PHP framework and I'm not going to use one.

Note: I'm using PHP 5.5

---

UPDATE: My current simplified version:

$model = new User();
foreach($row as $k => $v) {
  if(isset($model->$k)) {
    if(is_int($v)) {
      $model->$k = (int)$v;
    } elseif(is_string($v)) {
      if(($k == 'created_on' || $k == 'updated_on') && $v != null) {
        $model->$k = new DateTime($v);
      } else {
        $model->$k = $v;
      }
    } 
  }
}

Am I making the codes more complicated?

The reason I made the 2nd version is that I worried the 1st version expands if the $row expands.

Answer 1

I don't think there is much to simplify with your code.

• Used short array syntax, since you're running PHP 5.5.
• Renamed $model to $user: It increases readability + it's one less character.
• If-statement syntactical sugar brackets removed.

Code:

$row = [
  'id'         => 123,
  'username'   => 'Testing',
  'status'     => 1,
  'created_on' => '2014-09-20',
  'created_by' => 1,
  'updated_on' => null,
  'updated_by' => null
];

$user             = new User();
$user->id         = (int) $row['id'];
$user->username   = $row['username'];
$user->status     = (int) $row['status'];
$user->created_by = (int) $row['created_by'];

if ($row['created_on']) $user->created_on = new DateTime($row['created_on']);
if ($row['updated_on']) $user->updated_on = new DateTime($row['updated_on']);
if ($row['updated_by']) $user->updated_by = (int) $row['updated_by'];

I have the impression that your User class is just a container and everybody can edit its properties directly. You might wanna enforce some encapsulation here by using setters and getters. This way you can hide its internal properties and have room to validate any values being set and make sure a User object can never reach a state it wasn't meant to be in.

---

About your update:

Am I making the codes more complicated?

Yes! Do you really want to sacrifice all this readability just to get a more "simplified" solution? Really, it ends up over-complicating the solution instead.

Remember that if your code is made out of a bunch of complex and over-complicated logic because you wanted to "simplify" things, only a computer would understand that with no problem. In a week when you come back to look at your code, you will not be able to understand what's happening at a glance. It will take you a minute, and more often then not it makes you forget how the piece of code you're trying to understand fits in the bigger picture.

Martin Fowler said: "Any fool can write code that a computer can understand. Good programmers write code that humans can understand". And that goes especially if you work in a team. You should value readability over simplified code that over-complicates the solution. It will save you (and your team) a lot of time now and in the future.

The reason I made the 2nd version is that I worried the 1st version expands if the $row expands.

It's completely acceptable that if your user table gets a new usable column that you would also need to add an extra setter in your code for it. Generally it's something that would rarely happen often.

The solution in your post isn't even complete in regards to your concern. For example, what if you decided to update your table with a column modified_on? You still would have to append || $k == 'modified_on' to the if-statement expression and add the class property for this new column.

Answer 2

You fail to mention the reason why you convert a perfectly good array into an not so good object of the class User. A class is normally used to compartimentalize all aspects of something. A class consists out of properties and methods, both that is. You should preferably act on a class with its methods. I'm missing these concepts totally here. There's no instantiation of the class. It seems you left it out intentionally, but that's no way to ask a question. So, let's start with the instantiation:

class user
{
  private $row;

  public function __construct($row)
  {
    $this->$row = $row;
  }

  public getUsername()
  {
    return $this->$row['username'];
  }

  private getDate($key)
  {
    $date = $this->row[$key];
    return $date == NULL ? NULL : new DateTime($date);
  }

  public getCreationDate()
  {
    return $this->getDate('created_on');
  }
}

This example is not complete, it's just to show you what it might look like. More information about objects can be found here: http://code.tutsplus.com/tutorials/object-oriented-php-for-beginners--net-12762

The class can be used like this:

$user1 = new user($row);

In the class above the array is stored as a property, but you can choose to do some preprocessing of the array in the constructor. Note that only methods are public, and the array is not accessible from the outside. To simplify the class I've created a private method to deal with dates. This method can be reused by other methods that deal with dates.

Answer 3

I don't know if this can get simpler at all.

But instead of operating on a User object outside of the class, it would be better to move this initialization logic into the constructor. Let the User class constructor take an array like $row as parameter, and implement within itself the appropriate validations.

Typo?

The first assignment of $model->updated_by here looks like it may have been just an innocent typo:

$model->updated_by = (int)$row['updated_by'];
if($row['updated_on'] != null) {
  $model->updated_on = new DateTime($row['updated_on']);
}
if($row['updated_by'] != null) {
  $model->updated_by = (int)$row['updated_by'];
}