Написал класс но он выглядит как обычная функция, это нормально?

Question

[Антон Р.]

В общем для своего проекта (доска объявлений) решил запилить функцию продвижения объявлений как в Авито - выведение в топ. Для этого в таблице items добавляю два дополнительных поля к объявлению:
- promotion (может иметь значения 0 или 1)
- promotion_time (время продвижения, например 7 дней, задаем путем отсчета от текущего unix-времени + 604800 секунд, при проверке сверяем текущее время и записанное в базе)

И тут при выводе массива объявлений, для проверки продвигается ли объявление решил не пихать все проверки в свой процедурный код, а написал класс Promoter, который будет проверять является ли объявление продвигаемым и не истек ли период продвижения:

class Promoter {
	
	private $item_id;
	
	public function __construct($item_id){
		
		$this->item_id = $item_id;
		// $this->item_id будем дальше использовать при запросе в базу в методе checkPromotion().
	}
	
	public function checkPromotion(){
		
		// Проверяем у item поле в базе, 0 или 1
		// Если после первой проверки значение 1 -> Проверяем дату продвижения, 
		// если срок продвижения истек то записываем в поле БД "0" и возвращаем "0", если нет то возвращаем "1"
		
		return $period == true ? 1 : 0;
	}
	
}

$checkPromotion = new Promoter(1);
echo $checkPromotion->checkPromotion();

И тут весь юмор в том что у меня в этом классе всего одна функция ) Стоит ли ради этого городить такой огород?
P.S. Не судите строго за синтаксис и конструкцию, только постигаю ООП )

Comments

[Антон Р.]

Максим Федоров, ну типа "разделение" ответственности между классами, чтобы не получился один "СуперКласс" который умеет всё сам. По SOLID же надо делить.

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

Антон Р., надо по солид,
но говорить свое состояние -- это по солид

Answer 1

[Максим]

Отдельные классы это хорошая практика. Однако, саму проверку стоит размещать там, где она используется. В вашем случае это сделано не совсем а равильно. Проверку нужно поместить в класс, объявления, но при этом ссылаться на проверку класса промо.

Будет примерно так:

public function getPromotion(): ?Promotion
{
     return new Promotion($this->promotion_id, $this->promotion_time);
}

Далее помещаем метод проверки:

public function isPromoted(): bool
{
     return $this—>getPromotion()->isPromoted()
}

Если у вас данные для метода могут быть пустыми, тогда условие может выглядеть так

public function isPromoted(): bool
{
     return $this->getPromotion() ? $this—>getPromotion()->isPromoted() : false
}

Не претендую ни на что) Я делаю так) Создавать дополнительные классы это хорошо. Можно даже создать отдельный класс для Id. В типизации будет удобно работать. И понятно всем. Да и в целом не будет все в одном классе.

Comments

[Антон Р.]

Спасибо. Не совсем понял где какой код, то есть получается:
getPromotion() - эта функция должна быть в классе Promoter
isPeomotion() - а эта в классе Item ? (объявление)

[Максим]

Антон Р., мне не понятно что значит Item?) Методы и классы нужно называть своими именами. Так, чтобы их можно было читать как английский текст.

У вас получается так:
1. Класс объявлений (Advert)
2. Класс продвижений (Promotion)

Тогда получается так:

class Advert {

    private $promotionId;
    private $promotionTime;
....

   public function getPromotion(): ?Promotion
   {
       return new Promotion($this->promotionId, $this->promotionTime);
   }

   public function isPromoted(): bool
   {
        return $this->getPromotion() ? $this—>getPromotion()->isPromoted() : false
   }
}

class Promotion 
{

	private $id;
	private $date;

	public function __construct(int $id, DateTimeImmutable $date)
    {
        $this->id = $id;
        $this->date = $date;
    }

    public function isPromoted(): bool 
    {	//Проверка
    	return $result;
  	}
}

[Антон Р.]

Максим, ну да, у меня Item это единица сущности Объявление )

[Максим]

Антон Р., тогда я вам сделал) даже по DDD, а не только ООП) Приучайте себя назвать методы и классы своими именами. Если бы мы писали ваш пример на русском - это было бы так:

class Объявления {

    private $продвижениеИд;
    private $продвижениеВремя;
....

   public function получитьПродвижение(): ?Продвижение
   {
       return new Продвижение($this->продвижениеИд, $this->Продвижение время);
   }

   public function этоПродвигаемое(): bool
   {
        return $this->получитьПродвижение() ? $this—>получитьПродвижение()->продвигаемое() : false
   }
}

И так далее

[Антон Р.]

Максим, только зачем $продвижениеИд ? Ведь это не отдельная сущность, а поле в сущности Объявление.

[Максим]

Антон Р., это если у вас разные типы продвижения. Можно сделать продвижениеТип.

Типы:
- Выделить цветом
- Поднять в топ
- Повесить метку "горящее предложение"

и так далее

У вас получается тип продвижения "Выделить цветом" время "10 минут". Проверку делаете через метод этоПродвигаемое

В методе пишите если текущая дата - 10 минут < 0 тогда не показывать.. Ну как то так)

[Антон Р.]

Максим, а, точно, я как-то не подумал.

[Максим]

Антон Р., я ведь не знаю надо вам это или нет) Я лишь пример привел. Если это не надо, тогда передаете только время.

[Максим]

Антон Р., нашли решение?

[Антон Р.]

Максим, в каком смысле? Общий принцип более менее понятен, обкатывать буду чуть позже, сейчас решил "не прыгать по верхам", а начать с хорошей теории, нашел неплохую книжку (в дополнение к остальным):



Мне кажется моя проблема "не понимания" как раз в том что я перескочил несколько этапов изучения теории...

[Максим]

Антон Р., я говорю о том, что если наши ответ - пометьте решением) А то вопрос висит. Или скажите что ответ не полный. Дополним