Skip to content

Instantly share code, notes, and snippets.

/example.php Secret

Created May 7, 2016 14:06
Show Gist options
  • Save anonymous/2de443858b9567801c8834baf72132af to your computer and use it in GitHub Desktop.
Save anonymous/2de443858b9567801c8834baf72132af to your computer and use it in GitHub Desktop.
<?php
class StaticConstructor {
public static function me() {
return new static;
}
}
class SmsNotifier extends StaticConstructor {
public function notify(Event $event) {
echo "sms" . PHP_EOL;
// sending a sms ...
}
}
class EmailNotifier extends StaticConstructor {
public function notify(Event $event) {
echo "email" . PHP_EOL;
// sending an email ...
}
}
class SpaceNotifier extends StaticConstructor {
public function notify(Event $event) {
echo "space" . PHP_EOL;
// launching a rocket ...
}
}
class Event extends StaticConstructor {
public $name;
public $options = [];
}
final class Notifier {
public function notify(Event $event) {
SmsNotifier::me()->notify($event);
EmailNotifier::me()->notify($event);
SpaceNotifier::me()->notify($event);
return;
}
}
// ...
$notifier = new Notifier();
$notifier->notify(Event::me());
@rmabdulaev
Copy link

  1. Инициализация SmsNotifier, EmailNotifier, SpaceNotifier происходит в методе Notifier->notify что не соответсвует SOLID
  2. В случае если необходимо отправить только Sms или Email нужно будет писать новый метод или новый класс, класс не расширяем
  3. Если решили использовать инициализацию класса через статический метод ::me() класса StaticConstructor то к объявлению метода нужно добавить
    ключевое слово final - "final public static function me()" что бы предотвратить переопределение метод в потомках. Неоходимо добавить
  4. Метод notify() класса Notifier ничего не возвращает

PS
Мое мнение, тут лучше реализовать паттерн Observer

@katin-dev
Copy link

Мне кажется было бы лучше как-то так:

abstract class Notifier {
  abstract public function notify(Event $event);
  public static function create($type) {
    $className = ucfirst($type) . 'Notifier';
    return new $className();
  }
}
class EmailNotifier extends Notifier {
  public function notify(Event $event) {
    // sending an email ...
  }
}
class SmsNotifier extends Notifier {
  public function notify(Event $event) {
    // sending an sms ...
  }
}

class EventEmitter {
  private $subscribers;
  public function on($eventName, $handler) {
    self::$subscribers[$eventName] = $handler;
  }
  public function emit(Event $e) {
    foreach ($this->subscribers[$e->getName()] as $handler) {
      $handler($e);
    }
  }
}

// Обычно это будет как-то так: $this->app->ee->on("system.load", ...);
$ee = new EventEmitter();

// Потом где-то в bootstrap'е :
foreach (["sms", "email"] as $handlerName) {
  $ee->on("system.load", function (Event $e) use ($handlerName) {
    // Скорее всего sms, email, ... будут динамически меняться в зависимости от конфига или других параметров,
    // поэтому удобно держать их имена в массиве и подставлять в метод-фабрику
    Notifier::create($handlerName)->notify($e);
  });
}

Copy link

ghost commented Jun 2, 2016

Я бы переделал код так:

<?php
class StaticConstructor {
    public static function notify(Event $event) {
        $called_class = get_called_class();
        echo $called_class::CLASS_NOTIF . PHP_EOL;
    }
}

class SmsNotifier extends StaticConstructor {
    const CLASS_NOTIF = "sms";
}

class EmailNotifier extends StaticConstructor {
    const CLASS_NOTIF = "email";
}

class SpaceNotifier extends StaticConstructor {
    const CLASS_NOTIF = "space";
}

class Event {
    public $name;
    public $options = [];
}

final class Notifier {
    public static function notify(Event $event) {
        SmsNotifier::notify($event);
        EmailNotifier::notify($event);
        SpaceNotifier::notify($event);
    }
}
// ...
$event = new Event();
Notifier::notify($event);

@antony-efremov
Copy link

Во-первых, вынести в интерфейс вот это дело:
public function notify(Event $event)
Во-вторых, использовать явное инстанциирование через оператор new в методе Notifier::notify (доколе не используются фабрики);
В-третьих, наследование объектов использовать с целью создания недоклассов (на примере StaticConstructor) якобы для удобства кодинга - как минимум, нецелесообразно.

@velvetcat
Copy link

velvetcat commented Jun 29, 2016

А чего, собственно добивались авторы кода? Куча статики, наследование, ни одного интерфейса, отсутствие разделения на создание/использование - ради чего?

Если цель была в сокрытии факта наличия нескольких нотифаеров (хорошая цель), то:

<?php

// Используем этот интерфейс в потребителях
intefrace INotifier 
{
    public function notify (Event $event)
}

// конкретные нотифайеры
class SmsNotifier implements INotifier {}
class EmailNotifier implements INotifier {}

//нотифаер-обертка, предоставляет обычный интерфейс нотифаера, но сам ничего не делает 
class MassNotifier implements INotifier 
{
    /**
    * @param INotifier[] $notifiers
    */  
    public function __construct(array $notifiers) {}

    public function notify (Event $event)
    {
        /** @var INotifier $notifier */
        foreach ($this->notifiers as $notifier) {
            $notifier->notify($event);
        }
    }
}

Далее конфигурируем нотификации настройкой DIC, не трогая существующий код, в том числе для разных окружений.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment