так нормально писать?

[AmdY]

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

if (
		isset($_POST['notify'])
		&& (
		$oldOrder->summ() != $order->summ()
		|| $oldOrder->status != $order->status
		|| $oldOrder->sended != $order->sended
		|| $oldOrder->sizeof() != $order->sizeof()
		|| $oldOrder->postNum != $order->postNum
		)
) {
	/** @todo send to client email message */
	$this->core->sendEmail(
			$order->name,
			$order->email,
			false
	);
	// echo "<pre>"; var_dump($order);                            
}

 

[itprog]

это какой-то хаос-стайл %)

if (isset($_POST['notify']) && !$oldOrder->equals($order))) {
    /** @todo send to client email message */
    $this->core->sendEmail($order->name, $order->email, false);
    // echo "<pre>"; var_dump($order);                            
}

class order...
function equals($order) {
    return $this->summ() != $order->summ()
        || $this->status != $order->status
        || $this->sended != $order->sended
        || $this->sizeof() != $order->sizeof() 
        || $this->postNum != $order->postNum;
}

понятно и красиво.

ps: summ, sended - фу.

 

[tz-lom]

if(isset($_POST['notify']) &&
    ($oldOrder->summ() != $order->summ() ||
     $oldOrder->status != $order->status ||
     $oldOrder->sended != $order->sended ||
     $oldOrder->sizeof() != $order->sizeof() ||
     oldOrder->postNum != $order->postNum))
{
    /** @todo send to client email message */
    $this->core->sendEmail($order->name,$order->email,false);
    // var_dump($order);
}

 

[phprus]

craz
Если рассматривать расстановку скобок и операторов, то я пользуюсь таким-же стилем, особенно когда пишу сложные условия в SQL-запросах. С моей точки зрения он нагляден и удобен.
А если рассматривать сами по себе сложные условия в IF, то по возможности их стоит разделять на более простые, разносить по функциям и методам. Главное не переусердствовать и не разбить на разные функции логически связанные части условия.
В данном примере я бы скорее всего разбил условие на две функции, условно так: (is_valid_session_ip() || is_session_timeout()), если конечно это сложное условие применяется более чем 1 раз.

 

[Splurov]

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

Более того, стараюсь себя приучить любые условия длиннее 80-100 символов (от начала строки) форматировать подобным стилем.

 

[Ragazzo]

любые условия длиннее 80-100 символов
Validator бы помог ...а то так надоест писать

 

[Splurov]

Ragazzo
Validator?

if ($this->user->getType() == USER_DEVELOPER
		&& array_key_exists('editType', $_POST)
		&& in_array($_POST['editType'], array('task', 'task-comment'))) {
}

 

[zerkms]

Splurov
Во второй строке достаточно и короткого isset() без изысков в виде array_key_exists()

 

[Splurov]

zerkms
Да, всё равно две строчки останется.

 

[Ragazzo]

Splurov
Да...я лично не распространяюсь на условия где больше 3-4 проверок, и то для меня это не особо читабельно, выношу в отдельный метод например, либо смотрю где можно применить рефакторинг чтобы не читать громадные условия

 

[AmdY]

Splurov
$this->user->getType() == USER_DEVELOPER - почему не $this->user->isTypeDeveloper() , зачем разносить зону ответственности вынося её из модели.
валидация - это зона ответственности формы
if ($this->user->isTypeDeveloper() && $form->isValid())

p.s. На фоне многочисленных холиварах и шаблонах и orm, совсем забыли о спагетти коде в контроллере, ждём воскрешения triumvirat

 

[craz]

Splurov
ждём воскрешения triumvirat

ыы

 

[itprog]

AmdY
и что, на каждый тип вот так по геттеру? не вижу ничего плохого в $this->user->getType() == User::DEVELOPER

 

[AmdY]

itprog
это тоже самое, что делать писать в контроллере

echo "<div>Hello world!</div>";

такой же грех, как и пользоваться query builder в контроллере, а не вызывать готовый метод из модели.
молучается, что мы раскрываем внутренее устройство модели для контроллера, это затрудняет рефакторинг в случае изменения модели.
допустим предыдущий код может измениться и добавится ещё одна группа и проверка станет (у меня недавно был такой случай)

$this->user->getType() == User::DEVELOPER || $this->user->getType() == User::ROOT

неужени тогда менять код по всему проекту?

 

[itprog]

это совершенно разные примеры и "$this->user->getType() плохо потому, что глобалс/querybuild в контроллер/вывод в контроллере плохо" это не ответ %)

так а чем это будет отличаться от $this->user->isTypeDeveloper() || $this->user->isTypeRoot() ?
так же надо будет менять код по всему проекту

 

[AmdY]

itprog
потому что это функции модели знать своё состояние и сравнивать свои атрибуты. в случаем изменения мы просто правим один метод в модели, а не патчим контроллер. по сути данная проверка проверяет права, а мы в контроллере сравнивает тип, нужно часть type вовсе выбросить из названия метода
$this->user->isDeveloper() или $this->user->canEdit()

 

[whirlwind]

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

 

[Splurov]

Splurov
$this->user->getType() == USER_DEVELOPER - почему не $this->user->isTypeDeveloper() , зачем разносить зону ответственности вынося её из модели.
валидация - это зона ответственности формы
if ($this->user->isTypeDeveloper() && $form->isValid())

Тема о стиле форматирования кода

 

[AmdY]

whirlwind
я просто не вижу стыка отвественности. когда результат метода одного класса сравнивается с константой того же класса, почему эту логику не инкапсулировать в модели, для меня загадка. почему-то модель сводится к orm и завязывается только на работе с базой, а не полноценный элемент системы MVC.
Тогда даже оттестировать это можно прамо в модели, и не испытывать проблем в контроллере.