Nova2 refactoring #62
Conversation
|
|
||
| ```php | ||
| $np = new \LisDev\Delivery\NovaPoshtaApi2('Ваш_ключ_API_2.0'); | ||
| $np = new \LisDev\Controllers\NovaPoshtaApi2('Ваш_ключ_API_2.0'); |
There was a problem hiding this comment.
Не совсем понимаю зачем менять неймспейс, если прошлый вполне отражал свое назначение, а вот Controllers может вводить в заблуждение
src/Common/Format.php
Outdated
|
|
||
| class Format | ||
| { | ||
| public string $format; |
There was a problem hiding this comment.
Почему свойство публичное если у него есть сеттер и геттер?
src/Common/Format.php
Outdated
| * | ||
| * @return NovaPoshtaApi2 | ||
| */ | ||
| public function setFormat($format): Format |
| $this->throwErrors = $throwErrors; | ||
| $this | ||
| ->setKey($key) | ||
| ->setLanguage($language) |
| * | ||
| * @see https://my.novaposhta.ua/settings/index#apikeys | ||
| */ | ||
| protected $key; |
There was a problem hiding this comment.
пхп 8 позволяет типизировать свойства, чем советую активно пользоваться
| class AddressService | ||
| { | ||
| private PreparationDataService $preparationDataService; | ||
| private NovaPoshtaApiClient $novaPoshtaApiClient; |
src/Services/AddressService.php
Outdated
| */ | ||
| public function findNearestWarehouse($searchStringArray) | ||
| { | ||
| $searchStringArray = (array) $searchStringArray; |
There was a problem hiding this comment.
зачем его приводить к массиву если можно указать в хинтиге что это массив
| // Try to find current region | ||
| foreach ($areas as $key => $area) { | ||
| // Is current area found by string or by key | ||
| $found = $findByString |
There was a problem hiding this comment.
А вот если бы это вынести в метод 'IsAreaFounded(...): bool` то и комментарий не нужен был бы
| public function getArea($findByString = '', $ref = '') | ||
| { | ||
| // Load areas list from file | ||
| empty($this->areas) and $this->areas = (include dirname(__FILE__) . '/NovaPoshtaApi2Areas.php'); |
There was a problem hiding this comment.
жесткие зависимости и выполнение операций в проверках, плохая практика
|
|
||
| class Model implements ModelInterface | ||
| { | ||
| protected Model $model; |
There was a problem hiding this comment.
Довольно странная абстракция, модель хранит модель чтобы иметь иметь возможность выполнять с ней операции
2 participants
Nova2 refactoring