Меню
Что нового?

Давайте посмотрим мою ЦМС

cDLEON

cDLEON

Онанист РНРСlub
Ну ок. Сделаю ещё один вброс!
Нажмите для раскрытия...
Это и был вброс. Изначально. Попросил критику - получай. Посмотреть можешь на любом из распространённых фреймворков.
Своё покажу, после того как отрефакторю до, приемлемого для меня, уровня. Уже давно хочется, но постоянно находятся косяки, которые хочется исправить! :)))
 
craz

craz

Нестандартное звание
у меня есть че сказать... но не по коду...
 
  • Like
Реакции: Mols
Активист

Активист

Активист
Команда форума
Теме АП.

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

Прочитал тему сначала - ужас, хочу сказать следующее:
1. Пересмотрел подход к MVC.
2. Убрал ужасные агрегаторы
3. Полностью "переделал" объекты.

На том же примере модуля:
Контроллер уже такой: http://svn.local.prime-gr.ru/svn/baikalia.web.local/modules/tours/controller.php
Модель стала такая: http://svn.local.prime-gr.ru/svn/baikalia.web.local/modules/tours/model.php
Вид простой: http://svn.local.prime-gr.ru/svn/baikalia.web.local/modules/tours/view.php
Шаблоны модулей примерно такие: http://svn.local.prime-gr.ru/svn/baikalia.web.local/modules/tours/templates/ru/category.tpl.php
Объекты стали такими: http://svn.local.prime-gr.ru/svn/baikalia.web.local/modules/tours/classes/category.php
(наследство получают такое: http://svn.local.prime-gr.ru/svn/baikalia.web.local/core/classes/abstract.obj.php )

Вот еще например типичная "галерея" для любого объекта типа obj
http://svn.local.prime-gr.ru/svn/baikalia.web.local/core/classes/class.photos.php
http://svn.local.prime-gr.ru/svn/baikalia.web.local/core/classes/class.photo.php
 

Lirik

Новичок
Если бы убрать куда-нибудь с глаз эти ужасные свичи на 100 строк было бы возможно лучше
 

ferryman

.............
Если ООП подход, можно было бы и покрасивее сделать. Например,
PHP: 
Response::page404()
 
AmdY

AmdY

Пью пиво
Команда форума
ferryman
ООП это не слово класс, тем более в вашем статическом вызове вовсе нет объекта, а лишь неймспэйс.

Активист
очень-очень много кода
 

Lirik

Новичок
Активист
вообще методов куча,сделайте какой нибудь нормальный роутинг, я не в курсе всех тонкостей Вашей ЦМС, но это уж сильно режет глаза.Я вообще не понимаю почему многие начинают страдать велосипедизмом, имею ввиду что начинают придумывать свои корявые архитектуры, возьми какой-нибудь нормальный фреймворк, а лучше парочку, и оцени что и как, попробуй взять лучшее, не обязательно что взять и слизать все, просто взять на заметку пару моментов и реализовать у себя, хороший код редко сразу получается, в основном после кучи доработок, но используя хороший базис можно уберечь себя от такого говнокода
Со странице 404 и вообще с респонзами отдельная тема, все таки проще действительно это запихнуть в какой то метод или вызывать view соответствующий,
PHP: 
 $this->render('_error404');
Мне например нравится подход в Yii в данном случае, у них есть свой вид эксепшена CHttpExecption и он кидается например при не нахождении страницы
PHP: 
throw new CHttpException(404,'Указанная запись не найдена');
а дальше дефолтный их хендлер сам обработает и покажет ошибку
 
Absinthe

Absinthe

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

/core/core.php
Код: 
header("Content-type: text/html; charset=windows-1251", true);
Зачем геморойная кодировка? Чтобы потом перекодировать везде(что ты потом и делаешь)?

Код: 
error_reporting(E_ALL);
ini_set("display_errors", "on");
ini_set("date.timezone", "Asia/Irkutsk");
svn используешь? С другими девелоперами работаешь? Почему ЭТО не в конфигах?

Код: 
require("core/config.php");
if (!defined("CORE_CONFIG_LOADED")) {
	exit("Fatal error: confuration load error [core]");
}
RTFM http://php.net/require_once
Не забудь на сервере отключить отображение ошибок в страницу

Код: 
conv2cp1251
Зачем использовать внутри проекта 1251? о_О
Ну и название функции мне не нравится.

Код: 
$mainTemplate = "admin/default.tpl";
Файлы доступны с веба( http://baikalia.local.prime-gr.ru/core/templates/ru/admin/default.tpl ).
Тут либо вебсервер настраивай, чтобы файлы с веба не выполнялись(грязное решение), либо все выше документ рута переноси.
Часть шаблонов с .tpl.php, часть с .tpl - как я понимаю, ты по разным стилям прыгал и свой выбирал.

Код: 
core::getInstance();
core::getInstance()->core::getInstance()
core::getInstance()->core::getInstance()
14 штук - не много ли?
Не проще ли в переменную записать и использовать?
Это тоже: modulesInterface()


/core/config.php
Код: 
class cfg {
switch($host) {
	public function __construct() {
			case "baikalia.com":
				$this->MySQLUsername = "baikalia_v2";
ЧО? Не, я серьезно?
Зачем конфиг для нескольких сайтов? require_once 'local_settings.php' для такого и заигнорить его на репозитории.

/modules/tours/controller.php
Свич лишний. Раздели хоть по экшенам.

Код: 
->setUrlPart(isset($_REQUEST['urlPart']) ? $_REQUEST['urlPart'] : null)
->setText(isset($_REQUEST['text']) ? $_REQUEST['text'] : null)
->setRedirect(isset($_REQUEST['redirect']) ? $_REQUEST['redirect'] : null)
->setPublic(isset($_REQUEST['public']) ? $_REQUEST['public'] : null)
->setLider(isset($_REQUEST['lider']) ? $_REQUEST['lider'] : null)
->setPublic(isset($_REQUEST['public']) ? $_REQUEST['public'] : null)
Даже не захотелось сделать удобный request?

Код: 
 catch (Exception $e) {
Может исключения стоит типизировать?

Код: 
 $this->photos($model->category());
Вот этот момент(то, что внутри) не то, что до ООП не дотягивает, а даже до модульного.
Ну и переименовать методы не мешает.

Код: 
 $mysql->escape($row['urlPart'])."', '".$mysql->escape($row['url'])."', '".$mysql->escape($row['hotel'])."', '".$mysql->escape($row['stars'])."', '".$mysql->escape($row['photo'])."', '".$mysql->escape($row['text'])."', '".$mysql->escape($row['moduleTemplate'])."', '1', '".$mysql->escape($row['lider'])."', '".$mysql->escape($row['position'])."', '".$mysql->escape($row['lang']
ЧТО ЭТО? Ничего не хочется сделать? И это много где такое встречается.

/modules/tours/model.php
Код: 
 * Enter description here ...
Убрать.

Далее, я не понял какую задачу решает модель.
По сути ее можно раз в 5 сократить?


Код: 
 public function loadCategories() {
	$keys = array_keys($this->categories);
	foreach ($keys as $key) {
		unset($this->categories[$key]);
	}
	$this->categories = array();
ФЕЕРИЯ!

/modules/tours/view.php
Зачем на каждый новый шаблон новый метод?

/modules/tours/templates/ru/category.tpl.php
Наконец то что-то нормальное :)
Хотя я бы использовал альтернативный синтаксис - c endблок и двоеточиями. Ну и короткие пхп-теги выглядят получше. Это так, для верстальщика :)
 
makaron

makaron

Новичок
Absinthe написал(а):
Код: 
 public function loadCategories() {
	$keys = array_keys($this->categories);
	foreach ($keys as $key) {
		unset($this->categories[$key]);
	}
	$this->categories = array();
ФЕЕРИЯ!
Нажмите для раскрытия...
Facepalm.
/modules/tours/templates/ru/category.tpl.php
Наконец то что-то нормальное :)
Хотя я бы использовал альтернативный синтаксис - c endблок и двоеточиями. Ну и короткие пхп-теги выглядят получше. Это так, для верстальщика :)
Нажмите для раскрытия...
End и ":" одобряю, но короткие тэги - не хорошо, пусть и красиво во вьюхах выглядят. И вообще, лучше ставить strict mode.
P.S. 2Absinthe: отличная критика, и не лениво ж было столько пересмотреть :)
 

deepslam

Новичок
Еще маленькая рекомендация - вместо sizeof лучше использовать count )
 
Активист

Активист

Активист
Команда форума
Спасибо за комменты и потраченное время, отпишусь чуть позже подробнее, есть что сказать ))

Да
public function loadCategories() {
$keys = array_keys($this->categories);
foreach ($keys as $key) {
unset($this->categories[$key]);
}
$this->categories = array()
Нажмите для раскрытия...
Сделано для запуска деструкторов перед переопределением хранилица обектов, а также запуск в нужном порядке, при их использовании! Чем плох плох код? Плюс - я вот, не знаю, будет там теч или нет, но вдруг будет теч, а скрипт может быть долгожевущем, потому и принудительный unset, ибо возможно где-нибудь сохранится ссылка!
 
Absinthe

Absinthe

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

короткие тэги - не хорошо
Нажмите для раскрытия...
Часто такое слышал, но никогда никто не мог объяснить причину. Я не могу найти случая, когда они будут плохими.
Мою точку зрения подтверждает PHP 5.4, в котором они уже всегда on.

Сделано для запуска деструкторов перед переопределением хранилица обектов
Нажмите для раскрытия...
Читай про работу с памятью.
 
Активист

Активист

Активист
Команда форума
короткие тэги - не хорошо
Нажмите для раскрытия...
Короткие теги не использую из-за их зависимости от соответствующей конфигурационной директиве в PHP.INI, сначала их использввал, переучивался на длинные, обратно нет смысла.
 
Войдите или зарегистрируйтесь для ответа.
Сверху