Как сделать вывод коллекции объектов?

Question

[Max Ba]

class ProductCollection
{
  public $db;
  public $product;

  public function __construct(Product $product, DB $db)
  {
    $this->product = $product;
    $this->db = $db;
  }

  public function getAll():object
  {
    $data = [];
    
    foreach($this->db->getRows('SELECT `id`, `name`, `price` FROM `products`') as $v){
      
      //$product = new Product($v['id'], $v['name'], $v['price']); //вариант 1
      
      $product = new $this->product($v['id'], $v['name'], $v['price']); // вариант 2
	  
      $data[] = $product;
    }
    
    return (object)$data;
  }

}


$db = new DB;
$product = new Product;
$productCollection = new ProductCollection($product, $db);
var_dump($productCollection->getAll());

1. Не могу понять, надо ли добавлять в конструктор коллекции объект Product или лучше внутри инициализировать, как в закомментированном варианте 1?
2. Про вариант 2, странно, я уже создаю объект до передачи его в коллекцию, и потом еще внутри делаю new. Тут нет ошибки?

Answer 1

[ThunderCat]

Чет какая-то каша из топора и опилок...

1) Если это ProductCollection зачем туда передавать Product? Если бы это был просто Collection - это имело бы смысл.

2) Передавать DB в параметрах - такое себе, есть DI для такого, зачем этот мусор в конструкторе?

3) return (object)$data; вообще не айс, зачем было создавать массив, а потом его кастить? Создавайте сразу объект. Я вообще в данном случае не вижу смысла возвращать объект, верните массив и все, зачем эти касты? И зачем эти двойные присвоения?

$product = new $this->product($v['id'], $v['name'], $v['price']); // тут явная лажа с объявлением
$data[] = $product; // а тут явно лишний шаг, это можно было сделать строкой выше

4) new $this->product это вообще как??? У вас там не строка вроде, а объект...

Comments

[Max Ba]

1) понял, исправлю, спасибо
2) DI??? пока не понял, поищу инфо...
3) Если мне надо вывести коллекцию объектов, чтобы работала такая конструкция

foreach($collect->getAll() as $obj){
  echo $obj->name;
}

выводить в виде массива?

public function getAll():array
{
  $data = [];
  $rows = $this->db->getRows('SELECT `id`, `name`, `price` FROM `products`');
  foreach($rows as $v){
    $data[] = new Product($v['id'], $v['name'], $v['price']);
  }
    
  return $data;
}

[Max Ba]

4) пункт смущал даже меня :) понял, убрал это г.

[Max Ba]

2) сделать

public function initDB(DB $db){
  $this->db = $db;
}

?

[ThunderCat]

Max Ba,

выводить в виде массива?

да, у вас же вернется массив объектов, все будет точно так же, только без непонятно зачем используемого каста. И уберите это присваивание через лишнюю переменную, аж коробит...

DI??? пока не понял, поищу инфо...

Dependency Injection, собственно в более простом случае можно использовать Registry паттерн, хотя это более "текучая" конструкция, но для упрощенной схемы подойдет.

new Product($v['id'], $v['name'], $v['price']); - очень кривовато, либо хотя бы массив передайте в формате ключ-значение, либо дата трансфер обжект. Для начала подойдет массив, и я бы инициализацию вынес из конструктора во внутренний метод, типа $product->initByArray($dataArray);

[Max Ba]

ThunderCat, понятно. Спасибо большое.

[Дмитрий]

Max Ba, можно фетчить сразу в объект. Поля будут свойствами