0

I would like to know if is a good practice to declare a property of an object like:

$this->name = $name;

out of the function __construct

I am trying to build an object with data from a database table. But this object will be build only if the id is registered. I know that the __construct function always return an object so I can not get a false return. So I tried the following:

//test.php

$mod = new item($id);
if($mod->validate()) {
$item = $mod;
}

class item {

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

public function validate() {

        $db = new db('restaurants_items_modifiers');

        if($mod = $db->get($this->id)) {
            $this->price = $mod['price'];
            $this->name = $mod['name'];
            $this->desc = $mod['desc'];
            return true;
        } else {
            return false;
        }

    }
}

This will work but is a good practice to do it like this? or I should declare everything on the __construct function?

Tomas Lucena
  • 1,204
  • 1
  • 14
  • 31
  • One change I would make is to inject your database connection instead of making it inside the `validate()` method. – Rasclatt Jun 14 '17 at 03:20
  • Thats fine. You can see more here in this question [What is the function __construct used for?](https://stackoverflow.com/questions/455910/what-is-the-function-construct-used-for) – lgflorentino Jun 14 '17 at 03:20
  • Also, it might make more sense to inject the db into the `__construct($db)` and the `$id` into the `validate($id)` – Rasclatt Jun 14 '17 at 03:22

1 Answers1

1

Doing what you are doing is fine, but I think injecting the database into the construct and the id into the validate makes more sense. Creating a setId() may also be valuable:

class item
    {
        protected $id,
                  $db;
        # Inject the $db instead
        public function __construct(db $db)
            {
                $this->db = $db;
            }
        # Inject the id here
        public function validate($id = false)
            {
                if(!empty($id))
                    $this->id = $id;

                if($mod = $this->db->get($this->getId())) {
                    $this->price = $mod['price'];
                    $this->name = $mod['name'];
                    $this->desc = $mod['desc'];
                    return true;
                } else {
                    return false;
                }
            }
        # Create a method that can assign so you can reused the object
        public function setId($id)
            {
                $this->id = $id;
                # Return the object for chaining
                return $this;
            }
        # Have a method to get current id
        public function getId()
            {
                return $this->id;
            }
    }

# Create instance, inject db class
$mod = new item(new db('restaurants_items_modifiers'));
# Inject the id here
if($mod->validate($id)) {
    $item = $mod;
}

You can also reset the id doing this. They are essentially doing the same as injecting into the validate() but it depends on how much you want to be able to access $id down the line (maybe turning it private to lock it off from direct access may be required):

$mod->setId($id)->validate();
Rasclatt
  • 12,498
  • 3
  • 25
  • 33
  • Awesome this have a lot of sense for me... I need to learn a more about this injection part but I will definitely follow your advise – Tomas Lucena Jun 14 '17 at 03:51