D repin/hw 6 #1203
D repin/hw 6 #1203
Conversation
Nginx PHP-FPM Mysql Redis Memcache Composer
mysql/conf/my.cnf
Outdated
| @@ -0,0 +1,16 @@ | |||
| [mysqld] | |||
There was a problem hiding this comment.
Совет: не нужно добавлять те файлы которые не нужны для работы ДЗ.
www/src/Singurix/Chat/Client.php
Outdated
| while ($clientStarted) { | ||
| $consoleMessage = fgets(STDIN); | ||
| if (trim($consoleMessage) == 'stop') { | ||
| $clientStarted = false; |
There was a problem hiding this comment.
Тут можно сделать break и тогда можно избавится от переменной $clientStarted и от else
www/src/Singurix/Chat/Server.php
Outdated
|
|
||
| private function readMessage($sock): void | ||
| { | ||
| $socketMsg = false; |
There was a problem hiding this comment.
Немного запутанный код. Будет проще если:
- В цикле ждать соединения
- Как только получили соединение в другом цикле обрабатывать сообщения от клиента.
www/src/Singurix/Chat/Client.php
Outdated
| public function start(): void | ||
| { | ||
| $clientStarted = true; | ||
| $sock = (new SocketChat())->isServerRun()->create()->connect(); |
There was a problem hiding this comment.
Мы создаём инстанс друго-го класса. Лучше его создать где-то во вне и сюда инъектить как зависимость.
www/src/Singurix/Chat/Server.php
Outdated
| */ | ||
| public function start(): void | ||
| { | ||
| $sock = (new SocketChat()) |
There was a problem hiding this comment.
Мы создаём инстанс друго-го класса. Лучше его создать где-то во вне и сюда инъектить как зависимость.
www/src/Singurix/Chat/Client.php
Outdated
| if (trim($consoleMessage) == 'stop') { | ||
| $clientStarted = false; | ||
| } else { | ||
| socket_write($sock->socket, $consoleMessage, strlen($consoleMessage)); |
There was a problem hiding this comment.
Тут у нас протекла абстракция. У нас есть класс который абстрагирует нас от некоторых низкоуровневых вызовов методов сокета. Но не от всех, тут мы всё равно их используем. Стоит их перенести в класс SocketChat. Данное замечание валидно для всех мест.
www/src/Singurix/Chat/SocketChat.php
Outdated
| */ | ||
| public function __construct() | ||
| { | ||
| $configFile = '/app/config.ini'; |
There was a problem hiding this comment.
Лучше будет отделить конфигурацию в отдельный класс и инкапсулировать всю логику там. И сюда конфиг инъектить как зависимость.
www/src/Singurix/Chat/Config.php
Outdated
| { | ||
| return $this->config['socket']['file']; | ||
| } | ||
| } |
www/src/Singurix/Chat/Server.php
Outdated
| ->create() | ||
| ->bind() | ||
| ->listen(); | ||
| socket_set_nonblock($sock->socket); |
There was a problem hiding this comment.
Это надо делать внутри класса который работает с сокетом. Основной смысл создания класса - это инкапсуляция данных и методов работы с ними и абстрагирование пользователей от того как это работает. У Вас же получается это нарушено, пользователь, зачем-то знает как устроен класс. Чтобы понять как надо сделать, вместо конкретного класса SocketChat создайте интерфейс и везде используйте его вместо конкретной реализации.
There was a problem hiding this comment.
вместо конкретного класса SocketChat создайте интерфейс и везде используйте его вместо конкретной реализации
Вот такой подход не понял, можно поподробнее информацию или где-то почитать?
www/src/Singurix/Chat/Server.php
Outdated
| private function readMessage($sock): void | ||
| { | ||
| do { | ||
| if ($socketMsg = socket_accept($sock->socket)) { |
There was a problem hiding this comment.
Тут, как и в других участках кода, протекает абстракция. Чтобы исправить, сделайте свойство socket внутри SocketChat приватным и перепишите код.
No description provided.