[ Поиск ] - [ Пользователи ] - [ Календарь ]
Полная Версия: Код ревью ABC-фреймворка
Страницы: 1, 2, 3
chee
Надеюсь я не один буду критиковать код твина в этой теме, но думаю такую тему надо создать

Начну пожалуй:

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

Дано: инстанс класса который хранися в статическом свойств $abc на данный момент бесполезен, ты его никуда не передаешь и не видно что собираешься.

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

Дано:

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

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


Вопрос: Метод, явно делает чуть больше чем ничего, то есть в index.php можно тупо скопировать следущий код

self::$abc->process = new AbcProcessor($appConfig, $siteConfig);
self::$abc->process->startApplication();

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

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

Что я предлагаю: Сделать автозагрузку на основе composer'а без альтернатив, класс ABC\Abc удалить, процессор из него перенисти в index.php, остальное что было в классе ABC\Abc перенести в объект приложения и сделать не статическими методами.

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

_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
twin
Цитата (chee @ 20.04.2016 - 18:53)
Вопрос: Метод, явно делает чуть больше чем ничего


Вот в этом "чуть больше" и вся фишка. Изначальная идея такова, чтобы можно было запустить фреймворк с минимальными движухами. Скачал архив, распаковал и получил результат. Этот фреймворк задекларирован, как учебный. Начинать учить с комозера и всяких json'ов, это сразу гарантированно потерять 80% потенциальных потребителей. Я, как человек, несколько лет ведущий курсы PHP, это знаю точно.

А почему сразу всё не запихал в статику, так именно потому, что
Цитата
можно тупо скопировать следущий код...
с минимальными изменениями и он будет работать.
Потому что будет раздел "продвинутое использование". И там будет такой способ запуска фреймворка:
<?php

require __DIR__ .'/../vendor/autoload.php';

(
new ABC\Abc\Core\AbcProcessor)->startApplication();
C использованием композера и внутреннего синтаксиса. Этот момент не до конца реализован в продакшене, но уже испытан. Никакой сложности расшарить объект AbcProcessor и передать контейнер пользовательским скриптам. Просто я придерживаюсь принципа YAGNI. Всему своё время.

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

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

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

user posted image
kaww
Цитата (chee @ 20.04.2016 - 18:53)
Дано: инстанс класса который хранися в статическом свойств $abc на данный момент бесполезен, ты его никуда не передаешь и не видно что собираешься.

Используется в самом же классе. А вообще похоже на кривую попытку реализовать синглтон.
    protected function run($appConfig = [], $siteConfig = [])
{
$this->autoload = __DIR__ . $this->autoload;
$this->autoloadSelector();
self::$abc->process = new AbcProcessor($appConfig, $siteConfig);
self::$abc->process->startApplication();
}

Почему не так?:
         $this->process = new AbcProcessor($appConfig, $siteConfig);
$this->process->startApplication();

Есть еще такого типа реализация методов, несколько их:
    public static function process()
{
return self::$abc->process;
}

Что если не вызвать перед ним Abc::startApp(...)?
twin
Цитата (kaww @ 21.04.2016 - 03:22)
А вообще похоже на кривую попытку реализовать синглтон.

Почему попытку и почему кривую? biggrin.gif Это он родимый и есть.
Цитата (kaww @ 21.04.2016 - 03:22)
Почему не так?:
Дельное замечание, исправил
Цитата (kaww @ 21.04.2016 - 03:22)
Что если не вызвать перед ним Abc::startApp(...)?

Как это не вызвать? Такого не бывает.

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

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

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

user posted image
kaww
Цитата (twin @ 21.04.2016 - 03:35)
Почему попытку и почему кривую?

Можно создать новый объект, причем несколькими способами:
1. Через конструктор.
2. Клонированием
3. Сериализацией, затем десериализацией.
Цитата (twin @ 21.04.2016 - 03:35)
Такого не бывает.

Нельзя вызвать публичный статичный метод из любого места? А кто запретит?
chee
Цитата (kaww @ 21.04.2016 - 07:22)
Используется в самом же классе.

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

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

Ну дак сделай архив в котором будет композер и все что он сгенерил и подключил, положи этот архив на сайтик и давай на него ссылки этим 80%. А для тех кто шарит, сами скачают композер и установят твой проек терез пакежист. Не вижу в этой схеме чего то сложного и плохого.

_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
twin
Цитата (kaww @ 21.04.2016 - 03:41)
Можно создать новый объект, причем несколькими способами:

Мне нужен именно этот способ. Потому что дальше я буду для пользователей применять такие правила инициализации сервисов:
$pdo = Abc::getService('PDO');


Цитата (kaww @ 21.04.2016 - 03:41)
Нельзя вызвать публичный статичный метод из любого места? А кто запретит?

Цитата (chee @ 21.04.2016 - 05:42)
Я бы понял если бы там была одиночка, но сейчас это не она, нет там метода на подобии getInstance и не запрещены способы создать новый инстанс это класса.

Почему не запрещена. А это?

Цитата (chee @ 21.04.2016 - 05:51)
Не вижу в этой схеме чего то сложного и плохого.

Это в планах на будущее, я же сказал. У меня с такой реализацией есть проблемка. Которую пока красиво решить не смог. Собственно не сильно заморачивался, есть более важные дела.

А проблема в основном в шаблонах. Я не хочу в них прописывать немспейсы. Более того, я не хочу юзать в них классы. А потому у меня есть несколько обычных функций. Допустим вот эта генерирует URL.

Тогда в шаблоне все чистенько:
                    <a href="<?=href('/docs/setup') ?>">Установка</a>


Так вот, сложность красиво передать контейнер в функцию. Чесно говоря я об этом не думал пока.

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

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

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

user posted image
chee
Цитата (twin @ 21.04.2016 - 14:00)
Почему не запрещена. А это?

Очень специфично запрещено. Почитай пожалуйста про одиночк, а лучше посмотри код для этого паттерна, я еще раз скажу, здесь такой подход это излишнее усложнее, всё можно переписать на статику. У тебя сейчас по факту даже не одиночка, так как одиночка это всё таки объект, а не статический класс. У тебя какой-то мутант из подобия одиночки за статическим фасадом.

Цитата (twin @ 21.04.2016 - 14:00)
А проблема в основном в шаблонах. Я не хочу в них прописывать немспейсы. Более того, я не хочу юзать в них классы.

У тебя три выхода, либо передавать контейнер в функцию через аргумент, либо использовать глобальные переменные, либо поместить функции в класс и применить там внедрение зависимостей.

Цитата (twin @ 21.04.2016 - 14:00)
У меня с такой реализацией есть проблемка.

Как же так? в супер простом фреймворке могут быть проблемы?

_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
twin
Цитата (chee @ 21.04.2016 - 10:56)
Почитай пожалуйста про одиночк, а лучше посмотри код для этого паттерна

Я должен читать про одиночку??? Может вот это? biggrin.gif
Цитата (chee @ 21.04.2016 - 10:56)
У тебя какой-то мутант из подобия одиночки за статическим фасадом.
Называй как хочешь. Но это то, что мне нужно. Я конечно мог бы вернуть тот же объект. Но этого делать нельзя. Потому что приложение должно инициироваться только один раз. Этого вполне достаточно. Противное может привести к непредсказуемым последствиям.

Фреймворк задекларирован как учебный. Вот он и предупреждает об этом юзера.

Цитата (chee @ 21.04.2016 - 10:56)
У тебя три выхода, либо передавать контейнер в функцию через аргумент, либо использовать глобальные переменные, либо поместить функции в класс и применить там внедрение зависимостей.
Вот именно. И ни один из трех не подходит. Потому и не нашел пока красивого решения.
Цитата (chee @ 21.04.2016 - 10:56)
Как же так? в супер простом фреймворке могут быть проблемы?
Проблемы не в фреймворке, проблемы во времени. Я найду потом решение. Сейчас мне важнее другое. 80% важнее, чем 20. И отвлекаться я не хочу.


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

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

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

user posted image
chee
Короче я понял секрет успеха: говнокодишь такой, тебе говорят, что ты наговнокодил, а ты такой - это сделано для того что бы всем было понятно, ...., профит laugh.gif

_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
twin
Ну дык... Ты рассуждаешь с точки зрения:
Цитата
Есть два мнения. Моё и говнокод.


Я говорю - мне не нужен композер. Объяснил почему не нужен. Ты опять талдычишь:
Цитата (chee @ 20.04.2016 - 18:53)
Что я предлагаю: Сделать автозагрузку на основе composer'а без альтернатив
То есть я должен отказаться от большинства потенциальных потребителей только ради того, что ты без композера жить не можешь.

Я объяснил, что мне нужна возможность использовать два синтаксиса. Упрощенный на статике, и продвинутый в перспективе. Чтобы народ мог плавно набивать скилы. Ты опять за своё. Мутанты, не по правилам, говнокод. Да плевал я на правила, если они мешают мне достичь результата.

Ты вон по правилам делал, а все равно накосорезил уже дважды. С клонированием и с echо. А если глубже копнуть, то еще куча всплывет. Потому что твои правила вылились в 150 единовременно работающих классов. А в них черт ногу сломит.

Так что понятие "говнокод" - штука субъективная. По мне так твой оверинжениринг куда как больше воняет. smile.gif Сколько людей, столько мнений.

За критику по существу - спасибо. А эти каноны оставь для фанатиков. Я не из них.

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

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

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

user posted image
chee
Цитата (twin @ 21.04.2016 - 16:17)
Ты вон по правилам делал, а все равно накосорезил уже дважды. С клонированием и с echо. А если глубже копнуть, то еще куча всплывет. .

Можешь создать темку на ревью моей системки, я буду рад АДЕКВАТНОЙ критике.

Цитата (twin @ 21.04.2016 - 16:17)
Я объяснил, что мне нужна возможность использовать два синтаксиса. Упрощенный на статике, и продвинутый в перспективе. Чтобы народ мог плавно набивать скилы. Ты опять за своё. Мутанты, не по правилам, говнокод. Да плевал я на правила, если они мешают мне достичь результата.

Здесь не в правилах дело, этот мутант - переусложнение(оверинженеринг да еще и неудачный), несоответствие принципам YAGNI на которые ты молишся. Я не понимаю, зачем здесь объект, поясни почему ты не хочешь заменить все методы в этом классе на статические?

Цитата (twin @ 21.04.2016 - 16:17)
По мне так твой оверинжениринг куда как больше воняет.

Он воняет лавандой и персиками, а твой воняет говном laugh.gif



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

Ты серьезно думаешь, что то что я предлагаю с композером и архивом с уже сгенерированым композером автозагрузчиком это переусложенение?

_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
twin
Цитата (chee @ 21.04.2016 - 12:54)
поясни почему ты не хочешь заменить все методы в этом классе на статические?
АААААА!!! Я дебил. Вот что значит глаз замылился. Я уже не помню для чего делал, помню что вначале иначе не получалось. Может протупил просто чего. Конечно же, ты прав. smile.gif

Переделал на статику.

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

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

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

user posted image
Быстрый ответ:

 Графические смайлики |  Показывать подпись
Здесь расположена полная версия этой страницы.
Invision Power Board © 2001-2024 Invision Power Services, Inc.