Сверхбыстрая и компактная CMS Блоголёт - убийца WordPress

Да? Ну, пока у меня хорошее настроение, пройдусь немного по коду:

>index.php

ob_start();
.......
ob_end_flush ();

В нормальной концепции паттерна MVС вывод происходит в одном единственном месте - в конце отработки скрипта. Это просто аксиома любого нормального приложения.

И да, сразу - выводится в 1 месте не через перехват вывода, а нормально.

$Urlmap = &TUrlmap::Instance();

Для начала, в связи с особенности хранения обьектов в PHP5, а именно разнесения хранилищ на zVal и zObj куда помещаются объекты, все объекты передаются по умолчанию по ссылке.

Как по мне это все равно что азбука. Чтоб передать не по ссылке, надо явно это указать.

Дальше перешел посмотреть на объявление этого класса

>lib/urlmapclass.php

public static function &Instance() {
  return GetInstance(__class__);
 }

Читаем про нормальную реализацию паттерна Singleton

Дальше сразу в глаза бросается ужасный Code Styling, пройтитесь по коду каким-то IDE поправьте его.

Это кстати очень важно, но идем дальше в том же файле:

 public function Request($host, $url) {
  global $Options;
....
  global $paths;

Глобальные переменные недопустимы в концепции ООП. Как только видишь просто слово global - уже дальше можно не смотреть.

@filemtime

Ошибки надо обрабатывать и предупреждать, а не глушить. Тоже самое как с global - дальше уже можно было не смотреть.

    include($CacheFileName);
    return;

1) нету проверки а есть ли вообще такой файл.

2) return - это пять.

  eval('?>'. $s);

По всему приложению и дальше будут разбросаны eval - это вообще такое грубое нарушение что не знаю как описать. Просто eval в 99.9999% случаях нельзя ни при каких обстоятельствах использовать.

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

 global $paths;
  @unlink($paths['cache'] . $filename);
.....
    $this->unlink("$id-$i.php");

Вот такое повсеместно. Подавлние ошибок, без проверок, глобалы...

 public static function unsub(&$obj) {

У вас большое увлечение статическими функциями. Статические функции это само по себе нарушение ООП канонов и это не просто правило оно имеет под собой обоснование:

Почитайте про зависимости архитектуры. Правда Вы еще не поймете 90% написанного там, но будет куда стремится.

 if ($class == 'TTags') {
    eval('$result = "'. $html->addtag . '\n";');
   } else {
    eval('$result = "'. $html->addcategory. '\n";');
   }
  eval('$result .= "'. $html->{$form} . '\n";');
  } elseif (!$tags->ItemExists($id)) {
   eval('$result = "'. $html->notfound. '\n";');
  } else {
   $name = $tags->items[$id]['name'];
   if (!empty($_GET['action']) &&($_GET['action'] == 'delete'))  {
    if  (!empty($_GET['confirm']) && ($_GET['confirm'] == 1)) {
     $tags->Delete($id);
     eval('$result = "'. $html->successdeleted. '\n";');
    } else {
     eval('$result = "'. $html->confirmdelete . '\n";');
    }
   } else {
    eval('$s = "' . $s . '\n";');

Я не знаю, как этот код комментировать без матов, поэтому опущу.

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

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

Паблик методов должно быть ~2-3 в среднем а то и меньше в классе. А у вас их 10-15.

По сути, вы используете классы как namespace для своих функций.

Я за такой код, отправлял сразу же безоговорочно на стройку.

А самое страшное, то что ТС считает что пишет качественный ООП код и что остальные ламера. Если человек признается что он дурак, это уже задает вектор направления из дурака до умного. А тут клиника. Выносите.

HraKK, ну перестаньте =))) Вы же сами знаете, чтобы скрыть неумение создавать классические алгоритмы, разработчики пораждают тучи классов, которые просто скрувают это незнание. Ну "не круто" ща писать без дебрей ООП.

А еще, без обязательных наворотов типа , вместо того, чтобы b = a * 2, обязательно выкрутят b = a >> 1; Хотя второй вариант работает в 4 раза медленне первого (конечно идер речь не о компилируемх языках)

Линейное программирование никогда не будет признаком ушербности создателя. Ушербен тот крутой объектноориентированный кодер, который уже не понимает классики.

T.R.O.N,

Проблема заключается в том, что сейчас тонна ООП кода, тонна материалов, везде звучит ООП - это круто.

В том что ТС пишет такой код кстати и наша вина. Мы уже разжевав все принципы пишем заумные статьи о пользе ООП, волхвы возносим. Но! Мы то понимаем что ООП, это не код, это образ мышления. Невозможно, сменить образ мышления за пол года даже. А тут нахватавшись сливок сверху, начинают писать код который выглядит красивее функционального, типа то что ты говоришь

Ведь тут же многа красивых [зачеркнуто]буков[/зачеркнуто]слов. Смешно например тут смотреть в одном и том же файле подавление ошибок(@), не проверять вообще и не подавлять и тут же!!!! try{}catch{}

Чем сложнее инструмент, тем больше он возможностей дает сделать ошибок. Поэтому новичков, я бы вообще садил вначале на Cи, где нету ООП и он не прощает ошибок. *Улыбаясь вспоминаю ручной контроль за очисткой памяти, поиски memory leak*

Не чтоб скрыть, они их действительно не знают. А такое пишут из 2 причин:

1) Незная их, выдумывают всякое. Вот такое и получается зачастую.

2) Опять же незная, но где-то встречали такое, возможно в другом контексте - вот и сюда присобачивают.

А вам ТС, если вы еще немного адекватны, советую попроботвать начать программировать с исполльзованием методологии TDD. Хорошо вправляет мозги. Хотя для начала прочитайте: Макконелл "Совершенный код", после Мартин Фаулер "Архитектура корпоративных приложений" и после Мартин Фаулер "Рефакторинг". И параллельно пишите что-то с использованием TDD. Через год, откроете свой код и будите улыбаться.

Искринне желаю удачи.

HraKK: прошу прощения, за свой резкий выпад в пред сообщении, и большое спасибо за ревью - учту и исправлю, просто весь топик приходится сражаться и под вечер я уже готов был крыть всех матом, ладно бы еще по делу как у тебя, а то просто так писали... Все же php для меня не родной и вторичный язык (дельфи первый) b приходилось ложится в прокрустово ложе ограниченний php (нет строгой тпизации, нет виртуальных методов, непонимание некоторых механизмов управления памятью в php) наложили отпечаток. Я не претендую на особый статус моего кода, но есть все же чем гордится - модель событий, попытался это как бы портировать из дельфи в php исключительно для своего удобства (это после хуков в wp). Да, безусловно, код нуждается в в вылизавании - главное сейчас работает, и работает вполне удовлетворительно.

blogolet добавил 10.07.2009 в 16:22

RКстати по поводу включенного ob_start - это пришлось вклчить по следующим причинам: у меня вывод идет в одном месте и если не включить ob_start скрипт не отваливался додо закрытия сокета, например у меня на мобильнике страница открываллас 4 сек, и скрипт соответствено показывал 4 сек вполнения на сервере, после добавления ob_start скрипт вываливает все в буфер и отваливается, не дожидаясь закрытия сокета клиентом. Может быть это особености настройки моего сервера, но я описываю реальную ситуацию на конкретно моем сервере, так что с этим все ок.

Это не ограничения, а наоборот свобода даваемая PHP. Она то и убивает, мало людей способны самисебя контролировать.

З.Ы. но управление памятью однозначный плюс)))

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

ТС, каким блог клиентом пользуетесь?

Своим собственным, гордится правда им сейчас не могу (более года не обновлял, есть проблемы на многоядерных процах), скачать можно гу меня здесь:

http://blogclient.ru/

blogolet добавил 10.07.2009 в 20:50

HraKK: хочу ответить на твое ревью, так как по некоторым положениям ты не прав (про ob_start уже писал),

Далее смотри например:

$classname = 'MySomeClass';

$instance = $classname::Instance(); //не позволительная конструкция для php

$instance = GetInstance($classname); //единственный возможный вариант, так как

$instance = new $classname(); // возвратит новый экземпляр класса,

/*а чтобы возвратил существующий надо иметь либо статическую переменную, приэтом ее надо объявлять каждый раз в в ккаждом классе, а иначе если будет создан экземпляр родительского класса, то и значение статической переменной уже будет присвоено. Решение через статические переменные излишни будут дублировать код, и таким образом мое решение */

public static function &Instance() {

return GetInstance(__class__);

}

является практически единственным приемлемым.

далее:

@filemtime Ошибки надо обрабатывать и предупреждать, а не глушить. Тоже самое как с global - дальше уже можно

функция одновеременно делает две вещи: проверяет наличие файла и получает ремя, без @ не обойтись, либо разбивать на две функции, при этом для file_exists все равно надо ставить @ так как настройки предупреждений могут вывести сообщение об отсутствии файла (по мне это странное поведение функции, но оно было замечено).

все же перед строкой

include($CacheFileName);

стоит строка

if (($time = @filemtime ($CacheFileName)) && (($time + $Options->CacheExpired) >= time() )) {

и как я уже писал выше, функция filetime проверяет наличие файла.

eval('?>'. $s);

собствено исполняется сгенирированная страница, перед сохранением в кеш, откуда она будет include - подскажи иное решение? Полностью согласен, что eval вредная.

@unlink($paths['cache'] . $filename);

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

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

tag<?php echo get_some_value($param); ?>endtag

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

Далее "У вас все классы используются как обвертка над функциями, 99% методов класса паблик. Это говорит о том что вы совершенно не владеете основами объектного программирования.

Паблик методов должно быть ~2-3 в среднем а то и меньше в классе. А у вас их 10-15."

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

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

blogolet добавил 10.07.2009 в 21:37

Я написал ответы на некоторые вопросы по блоголёту у себя:

http://blogolet.ru/otvety-na-voprosy/

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

Что касается теста нагрузки на мой сервер - побойтись бога: 100 конкуррентных запросов мой ьболее чем скромный vds повесят сразу, у меня не выделенный сервер - пока все мои текущие потребности решает этот vds, плюс я даю бесплатно хостинг у себя нескольким проектам (не моим). Блоголёт здесь ни причем.

pattern Registry

Тогда уже пиши перед каждой функцией @, а то они тоже могут выдать

include

Считаешь сущность View (читай Template engine) недопустимым? Но при этом, нарушаешь MVC концепцию, и делаешь венегрет из PHP и HTML вместе, используя к тому же eval? Срочно в больницу.

gettext или банальный отдельный файл с массивом переводов.

Вы не поверите. Меня мало волнует согласны Вы с тем что земля круглая или нет.

Насчет обработчиков событий читаем pattern Observer или pattern Visitor

Извините, это Ваши проблемы Вы не находите? Я Вам делаю анализ и рецензию вашего УГ, а Вы мне еще указываете куда писать и как? Я бы Вам сказал сколько стоит мое время, да не буду.

не шутите так. Пхп спокойно выдерживает и 100 одновременных запросов. Википедия, если что на пхп.

А вот кстати файлы - очень большая нагрузка и медленность на шаред хостах. Основной тормоз в компах сейчас это винчестер и на шаредхостах головка скачет в таком ритме бешеном что это реальная проблема.

Почитал статью про синглетоны -ничего интересного и в сущности

function &instance() {

return Singleton::instance(__CLASS__);

}

что то мало отличается от моей, и совсем не отвечает на вопрос

$classname = 'some_class';

$instance = ???

По ходу дела ты так и не понял, в чем причина такого кода.

и зачем мне давать ссылку на include если вопрос идет о

$s = '<?php DoSomthing() ; ?>'; ..//вообще $s это целиком html страница с php вставками

eval('?>' . $s);

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

Я безусловно понимаю, что имею дело с центром вселенной, но все же мое предложение про пост на сайте блоголёта предназначено исключительно про работу блоголёта, тогда так как у нас спор идет про php.

gettext не подходит из за необходимости обязательного наличия софта третьих сторон - компилятор и редактор, но в конце концов я кажется уже придумал простое решение без eval

blogolet добавил 11.07.2009 в 18:24

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