Правила     Закладки     Карма    Календарь    Журналы    Помощь    Поиск    PDA    Чат   
        СМС-ки
   
Пейджер выключен!
Страницы: (3) 1 [2] 3  ( Перейти к первому непрочитанному сообщению )  
Фильтр авторов:    показать 
  скрыть
  Ответ в темуСоздание новой темыСоздание опроса

> Код ревью ABC-фреймворка
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 21.04.2016 - 12:57)
Ты серьезно думаешь, что то что я предлагаю с композером и архивом с уже сгенерированым композером автозагрузчиком это переусложенение?

Я не вижу в этом смысла. Я, если чесно, вообще не вижу смысла в композеровском автозагрузчике. Чем он так знаменит?

Мой автозагрузчик нормально работает, для чего лишние движения? Там есть опция, кто захочет, сам сделает. Я может попозже тоже сделаю. Когда более менее в порядок приведу. А то каждый раз генерить надоест.


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 21.04.2016 - 12:54)
Можешь создать темку на ревью моей системки, я буду рад АДЕКВАТНОЙ критике.

Я пробовал еще когда баттл был. Не получилсь по двум причинам.

Первая - я не смог поставить. А так не интересно.
И вторая - я запутался в твоих ветках. Смотришь код, разбираешь, выносишь свои замечания, а оказывается я не туда смотрел и ты уже всё переделал в другой ветке.

Я тогда еще сказал - ты приведи все в подобающий вид, тогда и обсудим.


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
chee  
 ۩  [x] Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Здесь живет
******

Профиль
Группа: Сын полка
Сообщений: 2043
Пользователь №: 38654
На форуме: 3 года, 7 месяцев, 13 дней
Карма: 47




Цитата (twin @ 21.04.2016 - 18:28)
Я тогда еще сказал - ты приведи все в подобающий вид, тогда и обсудим.

Ок

Идем дальше, по пути выполнения кода

https://github.com/abc-framework/abc-framew...onfigurator.php

Взляни на эту строчку

https://github.com/abc-framework/abc-framew...Abc/Abc.php#L43

и на метод

https://github.com/abc-framework/abc-framew...gurator.php#L40

То есть ты прокидываешь эти параметры бесчисленное количество раз и получается, что абстракция в итоге все равно протекла.

Исправить ситуацию можно так: Инициализацию конфигуратора перенсти из AbcProcessor в ABC\Abc, а уже в конструктор AbcProcessor передавать объект конфигуратора, плюс можно туда передавать и контейнер.

Примерно так

public static function startApp($appConfig = [], $siteConfig = [])
{
if (!empty(self::$process)) {
throw new \LogicException('Only one process');
}

self::$configurator = new AbcConfigurator($appConfig, $siteConfig);
self::$container = new Container;
self::$autoload = __DIR__ . self::$autoload;
self::autoloadSelector();
self::$process = new AbcProcessor($configurator, $container);
self::$process->startApp();
}


--------------------
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

Мой блог | SetCMS
PMПисьмо на e-mail пользователю
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
chee  
 ۩  Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Здесь живет
******

Профиль
Группа: Сын полка
Сообщений: 2043
Пользователь №: 38654
На форуме: 3 года, 7 месяцев, 13 дней
Карма: 47




На счет самого класса AbcConfigurator

https://github.com/abc-framework/abc-framew...gurator.php#L62
https://github.com/abc-framework/abc-framew...urator.php#L162
Вот эти методы должны быть вообще в другом классе, они вообще не связаны с управлением конфигурацией, а загружают какие-то языковые файлы и подключает перехватчики ошибок.


https://github.com/abc-framework/abc-framew...gurator.php#L26
Все кроме setConfig здесь лишнее, да и честно сказать весь метод setConfig должен быть в конструкторе, так как давать перезаписывать пользователю конфигурацию это на грани фола.


--------------------
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

Мой блог | SetCMS
PMПисьмо на e-mail пользователю
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 21.04.2016 - 17:45)
То есть ты прокидываешь эти параметры бесчисленное количество раз и получается, что абстракция в итоге все равно протекла.

Где именно протекла?
Цитата (chee @ 21.04.2016 - 17:45)
Исправить ситуацию можно так: Инициализацию конфигуратора перенсти из AbcProcessor в ABC\Abc, а уже в конструктор AbcProcessor передавать объект конфигуратора, плюс можно туда передавать и контейнер.

Не, ну вот тут ты точно не прав. У меня цель - запустить фреймворк двумя способами. Через класс Авс или через AbcProcessor (может переименую потом). Тогда получится два разных синтаксиса. Статика и динамика. Так что в Абс должен быть только запуск и обертки. Он сам по сути обертка.


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 21.04.2016 - 17:53)
Вот эти методы должны быть вообще в другом классе, они вообще не связаны с управлением конфигурацией, а загружают какие-то языковые файлы и подключает перехватчики ошибок.

Ну тут как трактовать Single responsibility principle. Конфигуратор в моей схеме, это не парсер конфиг. Это именно конфигуратор. Он задает настройки фреймворку, конфигурирует его. Устанавливает режимы ошибок, варианты роутинга, язык в конце концов. Там много еще чего может потом добавится. Так что все там на месте.

Цитата (chee @ 21.04.2016 - 17:53)
Все кроме setConfig здесь лишнее, да и честно сказать весь метод setConfig должен быть в конструкторе, так как давать перезаписывать пользователю конфигурацию это на грани фола.

А это задекларировано в концепции:
Цитата
Большая гибкость настройки конфигурации
и
Цитата
Фреймворк дает свободу программисту, не создавая каких-либо структурных ограничений и конвенций


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
chee  
 ۩  Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Здесь живет
******

Профиль
Группа: Сын полка
Сообщений: 2043
Пользователь №: 38654
На форуме: 3 года, 7 месяцев, 13 дней
Карма: 47




Цитата (chee @ 21.04.2016 - 16:03)
говнокодишь такой, тебе говорят, что ты наговнокодил, а ты такой - это сделано для того что бы всем было понятно, ...., профит

заменим слово понятно, на гибко


--------------------
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

Мой блог | SetCMS
PMПисьмо на e-mail пользователю
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 21.04.2016 - 19:07)
заменим слово понятно, на гибко

Причем тут гибкость...

Объясню немого. Я реалист и прагмтик. И прекрасно понимаю, что этот фреймворк не завоюет мир. Однако трачу на него время не напрасно, потому что хочу использвать его в своих курсах. У меня есть план построения занятий, а для него мне нужно эти два синтаксиса и как можно больше настроек. Дабы объяснять все поэтапно.

Так что просто прими это как данность, и перестань обзываться. smile.gif


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
chee  
 ۩  [x] Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Здесь живет
******

Профиль
Группа: Сын полка
Сообщений: 2043
Пользователь №: 38654
На форуме: 3 года, 7 месяцев, 13 дней
Карма: 47




Тем не менее, я буду дальше ревьюить, что бы показывать банальные ошибки.

https://github.com/abc-framework/abc-framew...ocessor.php#L48

sared?

---

Вызовы этого метода, плохо читаемым

https://github.com/abc-framework/abc-framew...ocessor.php#L46

Ты назвал метод сеттером, но при этом ничего не сеттишь, что бы понять что ты делаешь этим методом, надо бежать в сам метод и смотреть что он делает.

Правильным решением, на мой взгляд, будет сделать так

protected function setInContainer($alias, $className)
{
$container = $this->container;
$this->container->set($alias, function() use ($className, $container) {
return new $className($container);
});
}

protected function setInContainerAsShared($alias, $className)
{
$container = $this->container;
$this->container->setGlobal($alias, function() use ($className, $container) {
return new $className($container);
});
}

public function __construct($appConfig = [], $siteConfig = [])
{
$configurator = new AbcConfigurator($appConfig, $siteConfig);
$this->config = $configurator->getConfig();
$this->container = new Container;
$this->setInStorage('config', $this->config);
$this->setInContainer('AppManager', 'ABC\\Abc\\Core\\AppManager');
$this->setInContainer('BaseTemplate', 'ABC\\Abc\\Core\\BaseTemplate');
$this->setInContainerAsShared('Request', 'ABC\\Abc\\Core\\Request');
$this->setInContainer('Router', 'ABC\\Abc\\Core\\Router');
$this->setInContainer('RoutesParser', 'ABC\\Abc\\Core\\RoutesParser');
$this->setInContainer('Url', 'ABC\\Abc\\Core\\Url');
$this->setInContainerAsShared('Response', 'ABC\\Abc\\Core\\Response');
}


----

Этот метод, мне кажется вообще дикостью

https://github.com/abc-framework/abc-framew...ocessor.php#L74

Если тебе нужен регистр что бы перекидывать данные из одного места в другое, то сделай для этого специальный класс, да помести его инстанс в контейнер, а потом уж и бери его где хочешь и получай оттуда данные.


--------------------
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

Мой блог | SetCMS
PMПисьмо на e-mail пользователю
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 22.04.2016 - 06:05)
sared?

biggrin.gif biggrin.gif biggrin.gif
У меня одна из любимых поговорок - напугать пыльным мешком. Оговорка по Фрейду.
Цитата (chee @ 22.04.2016 - 06:05)
Правильным решением, на мой взгляд, будет сделать так

У меня привычка конструктор писать первым методом. Чтобы сразу было видно, есть он в классе или нет. Я просто изменю название метода. С английским как то не очень у меня, но наверное так addToСontainer. А вот со вторым подумать надо, как в два слова упихать смысл) Посоветуюсь с англичанами. smile.gif


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


И еще, я не понял, зачем полные имена? Эти методы используются только в том классе и только для классов ядра. Для того я их и делал.


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
chee  
 ۩  Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Здесь живет
******

Профиль
Группа: Сын полка
Сообщений: 2043
Пользователь №: 38654
На форуме: 3 года, 7 месяцев, 13 дней
Карма: 47




Цитата (twin @ 22.04.2016 - 11:11)
И еще, я не понял, зачем полные имена? Эти методы используются только в том классе и только для классов ядра. Для того я их и делал.

Я просто привел тебе вариант решения без именения концепции, если ты сделаешь метод addToСontainer, то это будет не так плохо.

Цитата (twin @ 22.04.2016 - 11:09)
У меня привычка конструктор писать первым методом.

Порядок методов тут вообще не имеет значения, я просто так оформил пример.


--------------------
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

Мой блог | SetCMS
PMПисьмо на e-mail пользователю
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 22.04.2016 - 11:00)
Порядок методов тут вообще не имеет значения, я просто так оформил пример.

Я поримаю, поэтому и подумал, что ты предлагаешь именно такое решение. Чтобы не далеко
Цитата (chee @ 22.04.2016 - 06:05)
бежать в сам метод и смотреть что он делает
biggrin.gif


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
chee  
 ۩  Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Здесь живет
******

Профиль
Группа: Сын полка
Сообщений: 2043
Пользователь №: 38654
На форуме: 3 года, 7 месяцев, 13 дней
Карма: 47




Мы продолжаем КВН

Решение крайне не прагматичное, при добавлении нового сервиса в этот класс придется добавлять константу.

Если ты хочешь сделать константы как контракт для других разработчиков, то наверное стоит этот блок констант стоит выделить в интерфейс, и уже оттуда брать их. Но я бы вообще не делал эти константы, ведь как я понял могут быть и другие сервисы по мимо стандартных.

Где свойство в который ты пихаешь вызов этого метода ?

Где свойство?

Где указание конкретного типа объекта или контракты в свойстве?

Судя по тому что я сейчас вижу, с автокомплитом кода сервисов у тебя все очень печально, да?


--------------------
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

Мой блог | SetCMS
PMПисьмо на e-mail пользователю
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
twin  
Дата
Цитировать сообщение

Пользователя сейчас нет на форуме



Глухой нуб
******

Профиль
Группа: Администратор
Почтальон группы
Сообщений: 16217
Пользователь №: 6543
На форуме: 8 лет, 10 месяцев, 13 дней
Карма: 304

Трезвый :
6 лет, 7 месяцев, 22 дня


Цитата (chee @ 9.08.2017 - 04:29)
Решение крайне не прагматичное, при добавлении нового сервиса в этот класс придется добавлять константу.
Ну это отсюда пошло. Мне тоже не особо нравится, но раз так положено, не переломятся пальцы дописать константу.

Что касается выноса в интерфейс - не вижу смысла в еще одной надстройке. Этот класс конфигурирует сервис, там константам самое место.
Для сторонних сервисов пока нет механизма. Планирую сделать что то типа декоратора. Они будут загружаться не этим классом.

За свойства спасибо, действительно косяк.
С типом тоже. Почти везде этот объект передается через конструктор, там указано в параметрах. Тут не доглядел по привычке.

А что не так с автокомплитом?


--------------------
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.

Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.

Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.

user posted image
PMСайт пользователяICQ
    0   Для быстрого поиска похожих сообщений выделите 1-2 слова в тексте и нажмите сюда Для быстрой цитаты из этого сообщения выделите текст и нажмите сюда
  Быстрый ответ
Информация о Госте
Введите Ваше имя
Кнопки кодов
Для вставки цитаты, выделите нужный текст и
НАЖМИТЕ СЮДА
Введите сообщение
Смайлики
:huh:  :o  ;) 
:P  :D  :lol: 
B)  :rolleyes:  <_< 
:)  :angry:  :( 
:unsure:  :blink:  :ph34r: 
     
Показать всё

Опции сообщения  Включить смайлики?
 Включить подпись?
 
1 Пользователей читают эту тему (1 Гостей и 0 Скрытых Пользователей)
0 Пользователей:

Опции темыСтраницы: (3) 1 [2] 3  Ответ в темуСоздание новой темыСоздание опроса