Задать вопрос
@phpcoder81
php

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

Парни, тренируюсь в написание простых классов. Недавно была задача написать генератор отчетов. Вот я и сделал. Посмотрите пожалуйста. Класс сильно сокращен, чтобы была понятна только суть.
1. "Круто" ли он написан и нет ли в нем ошибок?)
2. Что еще с ним можно сделать, чтоб дальше вникнуть в ООП?

class Otchet
{
	private $name;
	private $boss;
	private $directors = array();

	public function __construct($name)
	{
		$this->name = $name;
	}
	
	public function setBoss($name)
	{
		$this->boss = $name;
	}
	
	public function replaceBoss($name)
	{
		if(!empty($boss))
		{
			$this->boss = $boss;
		}
	}

	public function addDirector($name)
	{
		$this->directors[] = $name;
		
		$this->directors = array_unique($this->directors);
	}

    public function delDirector($name)
	{
		if(($key = array_search($name, $this->directors)) !== false)
		{
			unset($this->directors[$key]);
		}
	}
	
	private function generate_otchet_name()
	{
		return date('Y-m-d-H-i').'.txt';
	}
	
	public function save($filename = '')
	{
		if(empty($filename))
		{
			$filename = $this->generate_otchet_name();
		}
		
		$content = "Название отчета: ".$this->name."\n";
		$content .= "Директор: ".$this->boss."\n";
		if(count($this->directors) > 0)
		{
			$content .= "Зам директора: ".implode(', ', $this->directors);
		}
		
		if(file_put_contents($filename, $content))
		{
			return 'Отчет сохранен';
		}
		else
		{
			return 'Ошибка сохранения';
		}
	}
}

if(isset($_POST['name'], $_POST['boss'], $_POST['director']))
{
	$f = new Otchet($_POST['name']);
	$f->setBoss($_POST['boss']);
	
	if(is_array($_POST['director']) && count($_POST['director']) > 0)
	{
		foreach($_POST['director'] as $v)
		{
			$f->addDirector($v);
		}
	}
	
	echo $f->save();
}

<form method="post" action="#">
<input type="text" name="name" placeholder="Введите название отчета" /><br />
<input type="text" name="boss" placeholder="Ген директор" /><br />
<input type="text" name="director[]" placeholder="1-й зам директора" />
<input type="text" name="director[]" placeholder="2-й зам директора" />
<input type="text" name="director[]" placeholder="3-й зам директора" />
<input type="submit" value="Создать" />
</form>
  • Вопрос задан
  • 473 просмотра
Комментировать
Подписаться 1 Оценить Комментировать
Пригласить эксперта
Ответы на вопрос 3
HanDroid
@HanDroid
1) Почему функции и переменные с нормальным неймингом, а класс как "Отщит"?) Никаких f - называйте все своими именами.
2) Модель формы - выносятся в отдельный класс. Валидация полей в моделях формы. И вид тоже в отдельном файле.
3) Использование супер глобальных переменных POST GET - не допустимо, данные должны пробрасываться/инжектится в класс, а не использоваться внутри. Как правило они используются в контроллерах в рамках MVC.
4) Переводы и надписи выносятся тоже в отдельный файл i18n. Представьте ситуацию, вам завтра позвонили и сказали, надо сделать на немецком языке. Вы лезете в каждый класс, что бы это все исправить? А если классов 100 ?)
5) Метод Save можно превратить в апдейт одновременно. Если в метод добавлено Id то делаете апдейт записи, если нет, то создаете.
6) Я бы еще добавил исключений, но то уже такое, каждый делает где ему удобно.

Почему на босса есть сеттер, а на директора add Хотя это тоже сеттер ?)
Ответ написан
6 комментариев
6 комментариев
zorca
@zorca
Что делает это метод )))
public function replaceBoss($name)
{
if(!empty($boss))
{
$this->boss = $boss;
}
}
Ответ написан
4 комментария
4 комментария
@D3lphi 
public function setBoss($name)
  {
    $this->boss = $name;
  }


В данном случае, от этого метода - сеттера нет никакой практической пользы, он не проверяет ничего, не возвращает объект, чтобы можно было производить вызовы методов цепочкой $obj->a()->b(); В данном случае, целесообразнее использовать свойство напрямую.

Не нужно писать конструкции вот так:
if (...)
{
  //
}

Пишем так:
if (...) {
  //
}

Это не относится к классам, методам и функциям (в них скобку нужно писать на отдельной строке). Про это все, кстати, написано в PSR. Выберите стиль именования методов, чтобы он был один на протяжении всего кода, а то у вас в одном месте camelCase, в другом - under_scope.

if(file_put_contents($filename, $content))
    {
      return 'Отчет сохранен';
    }
    else
    {
      return 'Ошибка сохранения';
    }

Не надо возвращать строку! Возвратите true или false. И запомните: если что-то не так, бросайте исключение!
Ответ написан
19 комментариев
19 комментариев
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы
Вакансии с Хабр Карьеры
Разработчик PHP
Автомакон
от 206 000 ₽
PHP разработчик
Хабр • Москва
от 200 000 до 300 000 ₽
PHP разработчик Junior+/Middle
amoCRM • Москва
от 130 000 ₽
Ещё вакансии
Заказы с Хабр Фриланса
Дизайн страницы с каталогом вебинаров
11 янв. 2025, в 19:28
8000 руб./за проект
Проанализировать график и выдать результат по запросу
11 янв. 2025, в 19:19
15000 руб./за проект
С++, QML \ QT парсер игровых логов с выводом в клиентской части
11 янв. 2025, в 19:07
200000 руб./за проект
Ещё заказы