Правильно ли я использую класс?

Question

[Max Ba]

Парни, я создал класс отчет. Верно ли создал сам файл logs.php, где используется мой класс и нет ли грубых ошибок в самом классе?

class Logs
{
  protected $db;
  public $id;
  public $text;

  public function __construct($db){
    $this->db = $db;
  }
	
  public function create($text){
    $query = "INSERT INTO `logs` (`date`, `text`) VALUE (NOW(), ?)";
    $stmt = $db->prepare($query);
    if($stmt->execute(array($text))){
      return true;
    }else{
      return false;
    }
  }
	
  public function delete($id){
    $stmt = $db->prepare("DELETE FROM `logs` WHERE `id` = ?");
    if($stmt->execute(array($id))){
      return true;
    }else{
      return false;
    }
  }
	
  public function getAll($id = 0){
    if(!empty($id)){
      $query ="SELECT * FROM `logs` WHERE `id` = '".intval($id)."'";
    }else{
      $query ="SELECT * FROM `logs`";
    }
    $stmt = $db->query($query);
    return $stmt->fetchAll(PDO::FETCH_ASSOC);
  }
}

//logs.php ВОПРОС БОЛЬШЕ К ЭТОЙ ЧАСТИ КОДА

$logs = new Logs(DB:instance());

if(isset($_POST['add'], $_POST['text'])){
  if($logs->create($_POST['text'])){
    echo 'Запись добавлена';
  }else{
    echo 'Ошибка добавления';
  }
}

if(isset($_GET['del_id'])){
  if($logs->delete($_GET['del_id'])){
    echo 'Запись удалена';
  }
}

echo '<h2>Все отчеты</h2>';
echo '<ul>';
foreach($logs->getAll as $v){
  echo '<li>'.$v['date'].': '.$v['text'].'</li>';
}
echo '</ul>';

Comments

[pingo]

работает?

[Max Ba]

Да, только там заменил $this->db->prepare

Answer 1

[Максим Федоров]

Бегло посмотрев:
1. Зачем в классе объявлены публичные атрибуты id и text если они нигде не используются?
2. Вы везде используете $db->prepare, но локальная переменная $db не инициализирована. В этом коде Вы должны обращаться к атрибуту класса, т.е. $this->db->prepare
3. Зачем в каждом методе делать проверку эту проверку

if($stmt->execute(array($text))){
   return true;
}else{
   return false;
}

но какой в ней смысл если $stmt->execute и так возвращает boolean, проще написать просто
return $stmt->execute();
4. Да и вообще в случае возникновения ошибки лучше что бы метод не false возвращал а исключение бросал, а клиенты класса уже на него как-то реагировали
5. В методе getAll у параметра $id лучше поставить значение по умолчанию null

Comments

[Max Ba]

А на счет атрибутов, я честно сам не знаю. При создании классов обычно говорят, мол надо свойства создать. Но в данном примере я просто создал класс в котором действительно сам не понял зачем эти свойства)

Answer 2

[Anton Mashletov]

1) название класса в множественном числе никто не делает. просто Log.
2) используй PSR-3 Logger Interface, а не городи велик.
3) конструкции типа

if($stmt->execute(array($text))){
      return true;
    }else{
      return false;
    }

можно всегда сократить до
return (bool)$stmt->execute(array($text));