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

[Активист]

HraKK
Хорошо, хорошо))

А я знаю что код дурно пахнет) Думаешь не развивал бы тут холивар, когда пишу свою точку зренея, много продумываю, и задумываюсь, такие холивары по полочкам все разлаживают... ООП, как сказал *****, язык внутри языка, и уменее правильно его применять решает много проблем ))

-~{}~ 29.10.10 02:34:

Другой, но являющийся частью машины.

 

[Mols]

Маленько поздновато но по теме)))
В чём удобство фиг знает.
Наверное дело привычки.
Очень долго всё писал согласно варианту 2. (отсупы для кода а скобки всегда на следующей строке.)
Потом столкнулся с заказчиком который просил оформлять код следующим образом.
Управляющие конструкции так

if(expression){
    //code
}else{
    //code
}

foreach($a as $v){
   //code
}

а классы, методы функции так

function getName()
{
    //code
}

И что-то в этом есть.))
Сейчас так и делаю (если другое не оговаривается)
Так что дело привычки и не более.

 

[FB3]

Автор оригинала: Активист
метод двигатель() {
$this->двигатель = new Карбюраторный;
}

Двигатель не бывает карбюраторным, система подачи топлива может быть на основе карбюратора или инжектора

 

[itprog]

http://ru.wikipedia.org/wiki/Карбюраторный_двигатель

 

[FB3]

Автор оригинала: itprog
http://ru.wikipedia.org/wiki/Карбюраторный_двигатель

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

 

[Духовность™]

Я потратил 10 минут чтобы понять его целиком

не удивительно. это самый "сложный" и длинный контроллер из моего нынешнего кода.

И мне нравится идея выделять всё в функции, даже если сейчас мы вызываем ее только один раз. Это добавляет ясность в код и не отвлекает деталями. Для сравнения:


if (!$category->getId())
$redirect = new Base_Redirect();
$redirect->setHeader('action_failed');
$redirect->setType('alert');
$redirect->setMessage('element_does_not_exist');
$redirect->setRedirectUrl(array($this->getRequest()->getRequest()->getUriParts(0)));
return $redirect;
}

и

if (!$category->getId())
return $this->elementDoesNotExistRedirect();

Ясность в чем? Человек, который будет этот код поддерживать, должен ЗНАТЬ и ПОНИМАТЬ зачем существует туева хуча одноразовых методов типа elementDoesNotExistRedirect(). Ему нужно будет бегать туда-сюда по простыне кода, состоящего из мелких методов, что в конечном итоге приведет к путанице. Человек просто будет не в состоянии держать в голове все методы контроллера и помнить что они делают. Это анти-ООП - вы дробите целую систему, состоящую из множества объектов, на несколько подсистем. Всё равно, что взять собранный автомобиль, разрезать его пополам, а потом скрепить две половины одним основанием. Работать будет, но действие по разрезанию автомобиля на две части - бессмысленно. Результат тот же, а целостности - никакой.

Абстрагирование и инкапсуляция вещи хорошие, но они предназначены для другого. Все попытки дробить логику приложения (конкретных контроллеров) на кучу методов - это всё программистские привычки нацеленные на "повторное использование" и инкапсуляцию. Но фишка в том, что методы типа elementDoesNotExistRedirect() не будут способствовать повторному использованию кода, они вносят дополнительную сложность, сложность при поддержке и чтении кода.

Кто вообще сказал, что ОО-простыню читать сложно? Простыня читается в разы легче, чем куча "многоговорящих" методов (которые много-говорят одному лишь разработчику). Постоянно приходится код верх-вниз листать и смотреть, что там в каждом из этих методов. Ради чего все это вы делаете? Разбиение кода на функции/классы когда нужно создать независимую от контекста задачи функциональность - это одно. Но дробить логику приложения, тот конечный и ключевой код я смысла вообще не вижу! Я сейчас с таким кодом работаю, кстати - это очень тяжело!

>> $this->getRequest()->getRequest()

$this->getRequest() может возвратить объекты Request, Post, Get и Cookie.

 

[grigori]

я не считаю код, приведенный triumvirat, хорошим
а именно:
1. Имена методов и переменных не говорят о том, что они делают - это шифр для посвященных.
2. дергает объекты, которые работают с базой, сама собирает текст запроса к базе, работает с куки, присваивает значения полям объекта view.
мега-функциональность, не хватает echo и exit посередине для полноты картины.
3. Куча магических констант неопределенного смысла и происхождения передается в параметры.

"Простыня читается в разы легче", чем что?

что читается сложней, чем
$this->getView()->loadI18n('common/FrontendGeneral',
'advert/'.$this->getRequest()->getRequest()->getControllerName());
?

-~{}~ 28.10.10 23:50:

>бегать туда-сюда по простыне кода, состоящего из мелких методов

всего-лишь навести мышку, держа Ctrl, и прочитать декларацию с описанием

 

[AmdY]

grigori
да-да-да, хороший код, который гарантирует тебе незаменимость на работе.

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

 

[grigori]

большинство моих скриптов выглядит просто

<?php
_session_start();
if (empty ($_SESSION['admin'])){
    _header('Location: login.html');
    exit('Login required');
}

$location = _POST('location');
$state = _POST('state');
$distance = filter_pos_int('distance');

/**
* input validation here
*/

try{
    $DB = new DB();
    $location_id = $DB->getLocationIdByName($state, $location);
    if (!$location_id){
        exit ('Location not found');
    }
    $DB->addRestrictedLocation($location_id,$distance);
}catch(gksException $E){
    echo $E->getLogMessage();
    exit;
}

echo 'OK';

Скрипт вызывается через JSON.
И да, я помню спор на тему обертки для входящих данных, скрипты для web я вызываю только из web.

 

[Духовность™]

всего-лишь навести мышку, держа Ctrl, и прочитать декларацию с описанием

Это в теории. А что происходит, когда куча мелких методов пересекаются, вкладываются друг в друга? Я сегодня на работе сидел и выткал в подобный код. Один метод вызывает другой, другой в свою очередь третий и т.д. И все в рамках одного контроллера!

Имена методов и переменных не говорят о том, что они делают - это шифр для посвященных.

я всегда смотрю что делают методы с "говорящими" именами (такими например, как упомянутый тут elementDoesNotExistRedirect() ). Хотя бы потому, что "говорящие" имена, написанные другими разработчиками, мне ничего не говорят.

1. Имена методов и переменных не говорят о том, что они делают - это шифр для посвященных.
2. дергает объекты, которые работают с базой, сама собирает текст запроса к базе, работает с куки, присваивает значения полям объекта view.
мега-функциональность, не хватает echo и exit посередине для полноты картины.
3. Куча магических констант неопределенного смысла и происхождения передается в параметры.

ололо мой код большинства контроллеров выглядит так: http://pastebin.com/5ZeLMwrN

то что я до этого привел - исключительный пример =)

 

[korchasa]

Автор оригинала: triumvirat
я всегда смотрю что делают методы с "говорящими" именами (такими например, как упомянутый тут elementDoesNotExistRedirect() ). Хотя бы потому, что "говорящие" имена, написанные другими разработчиками, мне ничего не говорят.

А это не говорящее имя. Оно описывает существительное, хотя на самом деле действие.

Автор оригинала: triumvirat то что я до этого привел - исключительный пример =)

Главный косяк в том, что ты привел это неявное заполнение view, в случае ошибок валидации.

 

[HraKK]

большинство моих скриптов выглядит просто

потому что он не ООП

 

[korchasa]

Автор оригинала: triumvirat
Это в теории. А что происходит, когда куча мелких методов пересекаются, вкладываются друг в друга? Я сегодня на работе сидел и выткал в подобный код. Один метод вызывает другой, другой в свою очередь третий и т.д. И все в рамках одного контроллера!

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

1. Размазня
Возьмем привер Активиста. У нас замечена ошибка - не уходит письмо какому-то пользователю. Мы пока не знаем с чем это связано, он просто написал нам, что все письмо получили, а он нет. Что мы должны сделать? Смотрим метод который получает список пользователей. Все нормально. Ой, а оказывается чуть ниже мы проверяем "подписанность" пользователя, а наш, оказывается не подписан. Вот именно нечеткое распределение обязаностей и осложняет отладку.
Сравни 2 примера кога, и представь разницу в отладке

//вариант 1
$users = Foo::getUsers();
//10 строчек кода
foreach($users as $user)
{
  if($user->isSubscribed())
    $mail_service->sendEmail($user);
}

//вариант 2
$users = Foo::getSubscribedUsers();
//10 строчек кода
$mail_service->sendEmail($users);

2. Неявные передачи и большой контекст
Вернемся к твоему примеру. Ты вызываешь метод post(), который судя из названия обрабатывает POST-запрос. В случае, если данные не валидны, ты в глобальное(по отношению к методу) свойство view пишешь какие-то данные. Во-первых это не понятно из названия метода(из его названия вообще мало что понятно), во вторых это увеличивает количество необходимых для отладки знаний. Тебе нужно знать не только что он получил на входе и что должен отдать на выходе, но и куда он что запишет в процессе. Глобальные переменные - зло.
Опять же пример

$this->selectUsersForNotification();
$this->sendEmailsToSelectedUsers();

$users = $this->selectUsersForNotification();
$this->sendEmailsTo($users);

В первом примере мы имеем неявную передачу через хрен пойми что, во втором - отдельную коллекцию или массив $users, содержимое которой можем проверить при отладке.

 

[Активист]

У меня совсем другая логика.


foreach($users as $user)
{
if($user->isSubscribed())
$mail_service->sendEmail($user);
}

1. $user->supscribed (как атрибут) уже иже имеет булевой флаг, как ОБЪЕКТР, isSubscribed наводит на мысль, но ведь по сути это get'ер.

2. $mail_service->sendEmail($users);
т.е., ты предлагаешь передавать в метод sendEmail массив объектов юзер или объекта юзера? Т.е. хочешь заставить Service что-то знать о структуре объекта user? Задача Sevice - слать письма, он должен иметь свойства и и атрибуты связанные только с отсылкой писем.

-~{}~ 29.10.10 10:35:

> когда куча мелких методов пересекаются, вкладываются друг в
> друга?
Это не ООП, это сокращение кода, к ооп это не имеет значения. Часто люди спорят по этому не понимая смысл ООП.

-~{}~ 29.10.10 10:43:

FB3
Про двигатель.

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

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

 

[korchasa]

Активист
Ты не знаешь базовых вещей типа tell don't ask, разницы между структурой и интерфейсом, но безапиляционно вещаешь "истины". Можно я не буду тебе отвечать?

 

[zerkms]

korchasa
Нельзя, таки ответь ;-))

 

[Активист]

triumvirat
> $this->getRequest() может возвратить объекты Request, Post, Get
> Cookie.
Т.е., в обект типа "какой-то контролер (точно не помню какой), ты вставляешь рождение других объектов, отвечающий за
> может возвратить объекты Request, Post, Get
> Cookie.

Здесь правильнее:
ИнтерфесПрограммированияПриложения::получитьЕдинсвенныйОбъектТипаЗапросПост()->значение('полеВФорме');
Где тип обекта ЗапросПост уноследован от Запрос.

Либо.
ИнтерфесПрограммированияПриложения::КонструкторОбъектовГруппыСлужбы->создатьОбъектТипаЗапрос('пост')->значение('полеВФорме');

-~{}~ 29.10.10 10:55:

korchasa
> Ты не знаешь базовых вещей типа tell don't ask
Отвечай, что значит можно не буду. Заикнулся, раскрывай тему.

-~{}~ 29.10.10 11:09:

В контексте твоего контроллера, дожен буть еще один объект интерфейс, т.е.

контроллерМодуляПользователи->интерфейсСлужб->службаРеквист->пост, а вызываться он будет так

..
$this->serviceInterface->request->post->field('input_field');
$this->serviceInterface->getRequestService()->getPost()->getField('input_field');
$this->serviceInterface->getRequestPostService()->getField('input_field');

..

-~{}~ 29.10.10 12:34:

Кстати, Самое читабильное, создавать объект через new, если есть 100% уверенность, в том, что этот объект создаться, и для правильной его работы ничего делать не нужно, т.е., необходимость в интерфейсе отсутствует

 

[itprog]

> А это не говорящее имя. Оно описывает существительное, хотя на самом деле действие.

согласен, но с другой стороны редирект там - объект Redirect, т.е. существительное. есть предложения как переименовать этот метод?

я бы сделал наверно return new FailedActionRedirect("element_does_not_exist");

 

[craz]

ФЛЕЙМ!)

 

[Духовность™]

я бы сделал наверно return new FailedActionRedirect("element_does_not_exist");

какой смысл передавать ключ строки ошибки "element_does_not_exist" через параметр метода? Объект редиректа в моем случае принимать может тучу параметров. Тогда либо делайте интерфейса типа FailedActionRedirect($type, $string, $url, $params=array(), $hidden=1) и т.д.
Т.е. имея одну абстракцию Base_Redirect вы ещё добавляете абстракцию, тем самым усложняя понимание кода.

ИнтерфесПрограммированияПриложения::получитьЕдинсвенныйОбъектТипаЗапросПост()->значение('полеВФорме');

ИнтерфесПрограммированияПриложения в момем случае - контроллер. Это реализовано в одном из общих/абстрактных слоев.