Не пойму, в чем удобство такого метода написания кода

AmdY

Пью пиво
Команда форума
triumvirat
с моей точки зрения, твой код такая же портянка, точно такой же список неизвестно откуда взявший методов и классов.

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

korchasa

LIMB infected
Автор оригинала: triumvirat
Вот, например я выделяю post-обработку - http://pastebin.com/P9Y2Nn5r тут существующая "лапша" (в терминологии ОО-фанатов) меня вполне устраивает, т.к. в моем понимании это идеальный пример программы, когда я пишу только то, что требуется здесь и сейчас.
Очень непонятный run() на мой взгляд. Мне такое читать сильно проще.

ЗЫ: Этот код можно сильно сократить маленьким финтом - добавь в Validator_Chain метод который будет проверять правило только если в value пришло позитивное значение. Избавишся от кучи if'ов ;)
ЗЫЫ: Не бойся локальных переменных, они клевые.
 

Активист

Активист
Команда форума
AmdY, triumvirat, korchasa

А как вам читать мой код? Партянка? Понизте мне чсв ! ))

PHP:
<?php
class subscribeModel extends modulesModel_compact_02 {
	public function __do() {
		try {
			switch ($this->action) {
				case "admin/index.html?do":
					require_once(dirname(__FILE__)."../clients/classes/class.client.obj.php");
					require_once(dirname(__FILE__)."../clients/classes/class.clients.agregator.php");
					
					$this->data += array(
											"level0" => isset($_REQUEST['level'][0]),
											"level1" => isset($_REQUEST['level'][1]),
											"level2" => isset($_REQUEST['level'][2]),
											"subject" => isset($_REQUEST['subject']) ? substr(strip_tags($_REQUEST['subject']), 0, 0xff) : null,
											"body" => isset($_REQUEST['body']) ? $_REQUEST['body'] : null
										);
					
					$levels = array();
					if ($this->data['level0']) { $levels[] = 0; }
					if ($this->data['level1']) { $levels[] = 1; }
					if ($this->data['level1']) { $levels[] = 2; }
					
					if (!sizeof($levels)) {
						throw new Exception("Ошибка. Вы не выбрали ни одной группы пользователей");
					}
					
					if (empty($data['subject'])) {
						throw new Exception("Ошибка. Вы не указал тему письма");
					}

					if (empty($data['body'])) {
						throw new Exception("Ошибка. Вы не указали тело письма");
					}
					
					
					$clients = new clientsAgregator();
					$clients->appendByLevel($this->core->mysql, $levels);
					
					while ($obj = $clients->fetch()) {
						if ($obj->unsubscribed) {
							continue;
						}
						$this->core->tempalteEnenginear->assign("obj", $obj);
						$this->core->sendEmail(
												"{$obj->lastname} {$obj->firstname} {$obj->middlename}",
												$obj->email,
												$this->core->cfg['mailfrom']['text'],
												$this->core->cfg['mailfrom']['email'],
												htmlspecialchars($this->data['subject']),
												$this->core->tempalteEnenginear->fetch(
																						$this->core->getModuleTemplatePath($this->moduleName, "emails/email.tpl")
																						),
												true);
					}
 
				throw new Exception("OK", 200);
					
				default:
					exit("Module ".$this->moduleName.": fatal error: model don't know action: ".$this->action);
			}
			
		} catch (Exception $e) {
			switch ($e->getCode()) {
				case "200":
					$this->assignData();
					return true;
				case "":
				case "500":
					$this->assignData();
					$this->setError($e->getMessage());
					return false;
					
				case "404":
					$this->core->page404();
					exit();
			}
		}
	}
}

?>?>

Все документирована в классах, IDE отладает комплиты и подсказки.
 

craz

Нестандартное звание
мне вот что не понравилось... что то какой то не кашерный camelcase? а если точнее то мешанка...


А ваще имхо идеального кода не бывает, потому что не существует идеального проекта - нет 5 человек клонов одного разработчика, нет вменяемого заказчика, время выполнения проекта всегда меньше времени, которое потратил бы разработчик при всех равных условиях, бюджет всегда меньше, чем возможности команды и т.д. и т.п.
И поэтому как бы мы не кричали, что тут и там не так, как стоило бы делать, все равно нам приходиться это делать. Именно приходиться.
 

Adelf

Administrator
Команда форума
Активист
У тебя в одном месте собраны и разбор request и проверка входных, само действие и отлов исключений. Тому, что называется этот класс subscribeModel я уже не удивляюсь :)
Мы бы с тобой не ужились на одном проекте :)

Ой, ёёё.. у тебя метод заканчивается обязательной генерацией исключения? Теперь ты мой идеологический враг :)))
 

Активист

Активист
Команда форума
> копипаст детектед )
Оу шит... Попровимс))))

> compact_02
Компакт введен для поддержки старых разработок. Да, это напильник. Приходится поддерживать много проектов, в новых проектах этого нет, а обеспечить обратную совместимость нужно. Не считаю минусом. compact_02 - образован от расширения базового класса общего.

Adelf
Во первых, ты всю архитектуру не видел, во вторых, идиотское дробление вот этого:
> разбор request
> проверка входных
> само действие
В комманной разработки дает много минусов и не дает никаких плюсов. Здесь имхо, видно все действие модели. Смысл его дробления? Типа посмотрите, какой я нах ООП-ешник, вон сколько использую всего всего.

> генерацией исключения ... идеологический враг
А что не так? Почему враг, аргументы товарищ, аргументы ))

-~{}~ 28.10.10 15:43:

zerkms
Здесь исключение используется для ассигнации в шаблон массива $this->data, проблемы я в этом не вижу, или есть? После обработки метод все равно взывает return =)
 

Adelf

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

Но неужели только меня это коробит?
 

Активист

Активист
Команда форума
УЖ что что, но посмотрев этот код, как-то понятно как он работает, в вот посмотрел код предшевствиников, потребуется масса времени понять смысл этих методов и классов, а поскольку, время деньги, смысл теряется, да и вообще , смысла нет.
 

zerkms

TDD infected
Команда форума
Активист
Я не понимаю разницы, почему код

$this->assignData();
return true;

не мог быть вызван вместо выброса исключения.

Далее - исключения типа Exception - фи

Далее - твой валидатор сможет определить только одну ошибку. Т.е. при отсутствии subject и body одновременно - ошибка будет показана только для первого.
 

craz

Нестандартное звание
Автор оригинала: zerkms
в случае ожидаемого завершения - он должен заканчиваться return'ом, либо ничем.
ага я тоже так думаю...

исключение это же когда все пипец, дальше работать ничего не будет...

-~{}~ 28.10.10 10:51:

Активист
странно как-то, вы попросили понизить ЧСВ, вам его понижают, а вы как ежик колючки выставили, и по-моему не то что прислушиваться так вам наоборот поспорить...
 

itprog

Cruftsman
Автор оригинала: Активист
В комманной разработки дает много минусов и не дает никаких плюсов. Здесь имхо, видно все действие модели. Смысл его дробления? Типа посмотрите, какой я нах ООП-ешник, вон сколько использую всего всего.
т.е. твое лучше чем
PHP:
<?php
class subscribeModel extends modulesModel_compact_02 {
    public function __do() {                    
        $clients = new clientsAgregator();
        while ($client = $clients->fetch()) {
            if ($client->subscribed)
                $this->clientMailService->sendSunbscribeMail($client);
        }
    }
}
?>
?
не верю, я не понял что делает код, мне пришлось прочитать его 5 раз сверху вниз и только потом я понял что он делает.. он делает всё.
 

Активист

Активист
Команда форума
zerkms
Может быть так и привычней.

> твой валидатор сможет определить только одну ошибку
Это точно, но юзеру же написано что "поля обязательны к заполнению". Вполне можно сделать что-то вроде:
PHP:
class validator {
/** сетер */
public function append($mixed, $type, $length, $error) {

}
public function validate() {

}

/** тут гетеры ошибок типа getAll, getLast */

}
Мне минус к карме =)

-~{}~ 28.10.10 15:58:

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

itprog

Cruftsman
ну так это основной принцип разделения (?) - функции короче и классы как можно меньше. При правильном разделении, именовании и упорядочивании методов, код класса можно понять прочитав сверху вниз один раз (это что я и считаю хорошим кодом).

если мне надо убрать отчество зачем мне лезть во все эти дебри валидации, свитчей и условий?


еще интересно, у тебя этот switch ($this->action) { .. } во всех __do's?
 

zerkms

TDD infected
Команда форума
Это точно, но юзеру же написано что "поля обязательны к заполнению".
ну тогда раз написано - вообще убери валидатор нафиг! юзеры ведь всё читают и всегда действуют согласно инструкции )
 

Активист

Активист
Команда форума
zerkms
Ну это не юзеры, это админ-ка. Письма без боди и темы неприемлемы.
Здесь спорить не буду, согласился же на минус к карме =)))

> тебя этот switch ($this->action) { .. } во всех __do's?
Именно.

Т.е. модель на весь модуль одна, для удобочитаемости, в action передается часть урла (т.е., видя урл можно определить, какое действие).
 
Сверху