Полная Версия: Код-ревью SetCMS
| Цитата (twin @ 14.08.2017 - 09:48) |
| Но вот если потом вдруг понадобится выделить часть этого пути, начнется геморрой. |
dirname, basename и прочие функции ни кто не отменял. Они будут работать, насНколько я понял. И конечно же, ты уже пол страницы мозолишь инкапсулированый в объект код, который состоит из одной строки. Это уже не интересно.
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
Вообще то если кто то вызывает session_start() в коде, то не исключено, что где то в классах будут реально пользоваться фунциями session_status() и прочими функтиями которые могут привести при функциональном тестирование контроллеров к неожиданным эфектам...
Мне кажется, что лучше вместо прямого вызова функицй использовать Фаседе патерн, что позволит по сути менять функции сессии на фэйк функции.
А так для index.php или bootstrap.php всё вроде в норме.
Что мне в Зенде стало сильно не нравиться, так это параметры в виде масива и особенно если нет костант которые предлогают ключи к этому масиву... Это грузит и заставляет заглядывать в метод, что бы разобраться как он работает и что ему нужно.
_____________
↓↓↓↓↓↓↓↓↓↓
ответ может быть здесь
или в mysql_error();
Мне кажется, что лучше вместо прямого вызова функицй использовать Фаседе патерн, что позволит по сути менять функции сессии на фэйк функции.
А так для index.php или bootstrap.php всё вроде в норме.
Что мне в Зенде стало сильно не нравиться, так это параметры в виде масива и особенно если нет костант которые предлогают ключи к этому масиву... Это грузит и заставляет заглядывать в метод, что бы разобраться как он работает и что ему нужно.
_____________
↓↓↓↓↓↓↓↓↓↓
ответ может быть здесь
или в mysql_error();
| Цитата (SlavaFr @ 14.08.2017 - 11:10) |
| Мне кажется, что лучше вместо прямого вызова функицй использовать Фаседе патерн, что позволит по сути менять функции сессии на фэйк функции. |
мыслишь правильно, будут реализованы фейки и объект сессий, возможно фасад над ним.
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
| Цитата (chee @ 14.08.2017 - 06:04) |
| Это уже не интересно. |
Не будь эгоистом. :) Я же не в личку тебе пишу, нас люди читают. Может кому полезно будет.
| Цитата (chee @ 14.08.2017 - 06:04) |
| dirname, basename и прочие функции ни кто не отменял |
Нет конечно, но ты же не поставил ограничение, чтоб только под 7-кой работало. Потому я и говорю о чистоте кода. Согласись, это:
красивее этого
Продолжаем разговор. Вот это я не найду нигде... И вообще, это нормально там массив захардкодить? Я так понимаю, это перечень модулей, и он не конечный. Но ты же сам меня недавно попрекал, что
$parts = explode('/', $path);
$wanted = $parts[2];
красивее этого
$wanted = basename(dirname(dirname(dirname($path))));, даже если ты в цикл засунешь. К тому же отсчет тут идет с конца, что не прибавляет ни читабельности, ни функциональности. А если путь переменный, то геммор еще тот начнется. Ну да ладно, тебе с этим жить. :)
Продолжаем разговор. Вот это я не найду нигде... И вообще, это нормально там массив захардкодить? Я так понимаю, это перечень модулей, и он не конечный. Но ты же сам меня недавно попрекал, что
| Цитата (chee @ 9.08.2017 - 06:48) |
| изменение существующих классов это не ООП подход. |
И вроде бы константами надо... Кстати, у тебя интерфейс есть для этого класса. Хоть какое то оправдание ему было бы. :)
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
| Цитата (twin @ 14.08.2017 - 12:22) |
| $wanted = basename(dirname(dirname(dirname($path)))); |
это хотя бы понятнее, чем это
| Цитата (twin @ 14.08.2017 - 12:22) |
| $parts = explode('/', $path); $wanted = $parts[2] ; |
хоть и нет так выразительно.
| Цитата (twin @ 14.08.2017 - 12:22) |
| Продолжаем разговор. Вот это я не найду нигде... И вообще, это нормально там массив захардкодить? |
я давненько думал, как перенести это в конфигурация, но проблема в том что $appLegacy->set вставляется в середину массива. Если выносить в конфигурацию (а это идея у меня была), но тогда нужно массив с запускаемыми модулями, перебирать и смотреть соответствие ли называние модуля из элемента массива какому тэгу. И заменять на лету. если соответствует.
В будущем я уберу этот массив, в любом случае, потому что это не соответствует моим идеалам. Это осталось рефакторингма старого кода, когда я index.php рефакторил. Этих модулей вообще не было.
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
| Цитата (twin @ 14.08.2017 - 12:22) |
И вроде бы константами надо... Кстати, у тебя интерфейс есть для этого класса. Хоть какое то оправдание ему было бы.  |
Вообще не понял о чём речь?
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
| Цитата (chee @ 14.08.2017 - 08:40) |
| $appLegacy->set вставляется в середину массива |
А что это вообще и где оно, это свойство?
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
тут https://bitbucket.org/cheevauva/setcms/src/...pLegacy.php-126
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
Это же не свойство
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
twin, читай код, 2 последние строчки
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
Мнда... Очень прозрачно. :) Нет слов. GOTO попрозрачнее будет.
Ну ладно, ты мне скажи все-таки. Какой код в классе LegacyApp старый, а какой новый. Это совсем непонятно, потому что ты постоянно говоришь, что собираешься то-сё-другое потом переделать. Не понятно, ты переделываешь систему так, чтобы потом еще раз переделать? Или может цель эксперемента - разработать методику программирования через костыли?
Вот этот код готов или тоже временный костыль?
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
Ну ладно, ты мне скажи все-таки. Какой код в классе LegacyApp старый, а какой новый. Это совсем непонятно, потому что ты постоянно говоришь, что собираешься то-сё-другое потом переделать. Не понятно, ты переделываешь систему так, чтобы потом еще раз переделать? Или может цель эксперемента - разработать методику программирования через костыли?
Вот этот код готов или тоже временный костыль?
public function executeModule($module)
{
$indexPath = $this->filesystem->getPath(sprintf('modules/%s/index.php', $module));
$configPath = $this->filesystem->getPath(sprintf('modules/%s/config.php', $module));
if (file_exists($indexPath)) {
if (file_exists($configPath)) {
$this->includeFile($configPath);
}
$this->includeFile($indexPath);
} else {
$this->title = "Ошибка";
$this->text = $this->error;
}
}
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
| Цитата (twin @ 14.08.2017 - 14:33) |
| Вот этот код готов или тоже временный костыль? |
вероятно последние две строчки могу поменяться, остальное останется так как оно есть.
| Цитата (twin @ 14.08.2017 - 14:33) |
| Мнда... Очень прозрачно. Нет слов. GOTO попрозрачнее будет. |
Предложи вариант попроще. Как я должен забирать локальный контекст из старого кода? То что в loadContext лежит старый код, всего лишь вопрос времени. Главное сейчас, что этот код работает и изолирован.
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
| Цитата (chee @ 14.08.2017 - 10:43) |
| вероятно последние две строчки могу поменяться, остальное останется так как оно есть. |
Тогда тут почитай, полезно.
| Цитата (chee @ 14.08.2017 - 10:43) |
| Как я должен забирать локальный контекст из старого кода? |
Причем тут старый код... С этим то как раз все понятно, это в пределах метода. А то свойство вроде в новом прописано.
Ясно, это костыль. Трудный эксперимент)))
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
Ясно, это костыль. Трудный эксперимент)))
_____________
Если вам недостаточно собственных заблуждений, можно расширить их мнениями экспертов.
Нужно уважать мнение оппонета. Ведь заблуждаться - его святое право.
Настаивал, настаиваю и буду настаивать на своем. На кедровых орешках.
twin, на счет нулевого байта, код executeModule править не буду. Просто сделаю белый список модулей и буду проверять переменную set соответствует ли она какому-либо элементу в этом списке. Вообще таких мест видимо много, надо везде делать белые списки - блоки, шаблоны.
| Цитата (twin @ 14.08.2017 - 15:35) |
| Причем тут старый код... С этим то как раз все понятно, это в пределах метода. |
Потому что это копипаста из старого кода.
| Цитата (twin @ 14.08.2017 - 15:35) |
| А то свойство вроде в новом прописано. |
Оно прописано там не потому что какой-то код из вне полагался на это свойство, а потому что я объявляю это и другие свойства (из старого кода), которые используются в локальном контексте. Добавил кстати свойство, я недавно https://bitbucket.org/cheevauva/setcms/comm...d5585428d719aa4, так что его там не было.
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
Завел задачу на последние замечания https://bitbucket.org/cheevauva/setcms/issues/1/applegacy
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации
Здесь расположена полная версия этой страницы.