Exit и компания

← →
Джо © (2006-03-23 19:13) [0]

По следам недавних священных войн.

Вот написал только что код, который привожу ниже и, вспомнив о строгих противниках Exit"а, озаботился: ясен ли сей код, можно ли сделать его яснее без Exit и — имеет ли он право на жизнь? :)

Прошу более опытных коллег прокомментировать его по существу сабжа.

var Ds: IDataSource; begin if SelectDataSource(Ds) then begin if (Ds.HasOptions) and (not Ds.SetOptions) then Exit; CreateDocFromSource (Ds); end; end;

Пояснения.
SelectDataSource — вызывает диалоговое окно выбора некоего набора данных, возвращает True, если юзер совершил этот выбор, записывая в Ds ссылку на этот самый набор. В противном случае возвращает False.
Ds.HasOptions — этот метод возвращает True, если набор данных требует предварительной настройки своих параметров пользователем.
Ds.SetOptions — вызывает диалог настройки набора данных. Возвращает True, если пользователь подтвердил настройки в диалоговом окне.

Вроде всё.

← →
Anatoly Podgoretsky © (2006-03-23 19:15) [1]

if not ... then CreateDocFromSource (Ds);

← →
Jeer © (2006-03-23 19:20) [2]

var
Ds: IDataSource;
begin
if SelectDataSource(Ds) and Ds.HasOptions and (not Ds.SetOptions) then Exit;
CreateDocFromSource (Ds);
end;

// Чем ближе к С - тем лучше.
// "Один Exit в дополнение к end; - это всего лишь способ имет два выхода, на
//всякий пожарный"

← →
wicked © (2006-03-23 19:22) [3]

> if not ... then CreateDocFromSource (Ds);

тогда уж по де Моргану расписать:
not (A & B) = not A | not B

то есть
if not ((Ds.HasOptions) and (not Ds.SetOptions)) then
будет как
if not Ds.HasOptions or Ds.SetOptions then

ку?...

← →
Anatoly Podgoretsky © (2006-03-23 19:33) [4]

Это уже домашнее задание, суть не выходить, а выполнять.

← →
Джо © (2006-03-23 19:56) [5]

> [2] Jeer © (23.03.06 19:20)

Э... нет — Exit не устранен ;)

> if not Ds.HasOptions or Ds.SetOptions then

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

← →
Джо © (2006-03-23 19:57) [6]

> меню

меня.

← →
Sergey Masloff (2006-03-23 20:03) [7]

Джо © (23.03.06 19:56) [5]
Я бы заменил Ds.HasOptions методом Ds.NeedOptions с обратным действием так как not тоже плохо. Но не знаю в каком контексте чаще вызывается.

← →
Жуков Олег (2006-03-23 20:09) [8]

Я бы вот так написал. Не хочу зависеть от всяких опций компилятора
var
Ds: IDataSource;
begin
if SelectDataSource(Ds) then
if Ds.HasOptions then
if Ds.SetOptions() then
CreateDocFromSource (Ds);
end;

← →
Жуков Олег (2006-03-23 20:12) [9]

Упс, ошибся, это же другие действия. Тогда так, или с флагом
var
Ds: IDataSource;
begin
if SelectDataSource(Ds) then
if Ds.HasOptions then
begin
if Ds.SetOptions() then
CreateDocFromSource (Ds);
end
else
CreateDocFromSource (Ds);
end;

← →
Джо © (2006-03-23 20:24) [10]

> [7] Sergey Masloff (23.03.06 20:03)
> Джо © (23.03.06 19:56) [5]
> Я бы заменил Ds.HasOptions методом Ds.NeedOptions с обратным
> действием

Так вроде ж HasOptions и NeedOptions — функциональные синонимы? Сорри, если не понял идею — нахожусь в несколько "переработавшемся" состоянии :)

> [9] Жуков Олег

Нет, спасибо, такой монстрик не устраивает категорически. :)

← →
Суслик © (2006-03-23 20:33) [11]

я exit не использую.
пользую только break в циклах.

← →
Sergey Masloff (2006-03-23 20:47) [12]

Джо © (23.03.06 20:24) [10]
Ну а я тебя не понял. Видишь уже плохое имя

Вообще в твоем конкретном случае я бы скорее всего написал бы
function IDataSource.SetOptions : boolean; begin if HasOptions then begin // Your code end else Result := True; end;

и код был бы

var Ds: IDataSource; begin if SelectDataSource(Ds) then if Ds.SetOptions then CreateDocFromSource (Ds); end;

Но возвращаясь к твоему варианту я бы понимал
<Has> и <Need> как противоположности чего-то. Ну то есть вообще-то какой-то атрибут поддерживается но его не установили. И его нужно установить
А вот то что его вообще нет - ну нет у него вообще параметров
тогда что-то типа <Supports>

Но это мое глубокое ИМХО и при убедительных доводах я его даже готов поменять ;-)

← →
Джо © (2006-03-23 20:55) [13]

[12] Sergey Masloff (23.03.06 20:47)
А, понял идею насчет того, чтобы DataSource сам решал, нужно ли ему настраивать опции. Спасибо, полезная идея :)Правда, немного размытой семантика метода SetOptions получается. Т.е, сложность логики просто перемещается из одного места в другое ;)

← →
Джо © (2006-03-23 20:57) [14]

> [11] Суслик © (23.03.06 20:33)
> я exit не использую.

А я использую :) Люблю с помощью него осуществлять проверку пред-условий в методах.

← →
Sergey Masloff (2006-03-23 21:03) [15]

Джо © (23.03.06 20:55) [13]
Ну внутрь она перемещается. Читать-то обычно внешнее только приходится если проблем нет ;-) Вот и полегче чуть-чуть.

← →
Джо © (2006-03-23 21:12) [16]

> [15] Sergey Masloff (23.03.06 21:03)

Ну, в моем случае жизнь легче не становится, как раз наоборот.
Потому, что код в [0] пишется один раз, а именно — в обработчике TAction главного приложения, а вот код реализации этих самых IDataSourc"ов будет писаться много раз. И постоянное повторение того, что в [12] для IDataSource.SetOptions будет несколько утомительно :)
Сейчас каждый имплементатор интерфейса просто определяет такие нехитрые методы, отвечающие их названиям
function TSomeDataSourceImplementer.SetOptions : boolean; begin Result := // есть или нет опции end; // для тех, что имеют опции, код такой function TSomeDataSourceImplementer.SetOptions : boolean; begin Result := ShowOptionsSetupDialog = mrOk end; // для тех, что не имеют, можно вообще оставлять пустое тело метода
Проще, кажется, придумать нельзя. (?)

В общем и целом, остановлюсь, наверное, на [0] варианте, мне вроде так легче читается, тем более, что код пустяковый, запутаться трудно ;)

Спасибо всем, но с удовольствием послушаю еще критику и соображения по поводу читабельности, Exit и прочего :)

← →
Джо © (2006-03-23 21:12) [17]

> [16] Джо © (23.03.06 21:12)

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

← →
Суслик © (2006-03-23 21:27) [18]

> Джо © (23.03.06 20:57) [14]
> > я exit не использую.
>
> А я использую :) Люблю с помощью него осуществлять проверку
> пред-условий в методах.

это, пожалуй, ед-ое исключение, которое я тоже иногда себе позволяю :)

← →
Marser © (2006-03-23 21:30) [19]

> [14] Джо © (23.03.06 20:57)
> > [11] Суслик © (23.03.06 20:33)
> > я exit не использую.
>
> А я использую :) Люблю с помощью него осуществлять проверку
> пред-условий в методах.

Я тоже.

← →
Геро (2006-03-23 23:43) [20]

> А я использую :) Люблю с помощью него осуществлять проверку
> пред-условий в методах.

Ага, особенно когда процедура уже написана и помещать все в лишний begin...end совсем нет желания :)

← →
Marser © (2006-03-23 23:57) [21]

> [20] Геро (23.03.06 23:43)

И это очень весомо, у меня при обработке пакетов структура подпрограмм бывает достаточно сложная и нет смысла увеличивать вложенность бегин-эндов там, где можно обойтись exit :-)
Тем более, не библиотеку пишу...

← →
Джо © (2006-03-24 00:15) [22]

Вот есть одна хорошая весчь в C, которой нет в Паскале, а именно — процедура return с кодом возврата...

← →
Геро (2006-03-24 00:21) [23]

> И это очень весомо, у меня при обработке пакетов структура
> подпрограмм бывает достаточно сложная и нет смысла увеличивать
> вложенность бегин-эндов там, где можно обойтись exit

Да, и наверное, это правильно.

← →
Игорь Шевченко © (2006-03-24 00:56) [24]

> if (Ds.HasOptions) and (not Ds.SetOptions) then
> Exit;
>
> CreateDocFromSource (Ds);

if not Ds.HasOptions or Ds.SetOptions then
CreateDocFromSource (Ds)

Вроде так. Относительно ясности - или набор не имеет настроек или настройки заданы пользователем, тогда выполнять CreateDocFromSource.

А первый if (if select...) я бы оставил. Для ясности.

Код вижу первый раз, Exit не очень люблю, но использую

← →
Джо © (2006-03-24 01:27) [25]

> [24] Игорь Шевченко © (24.03.06 00:56)

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

Еще раз спасибо всем за помощь.

Джо © (24.03.06 01:27) [25]

Хочешь критик - их есть у меня.

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

> [26] Игорь Шевченко © (24.03.06 01:36)
> На мой взгляд, метод перегружен функциональностью, хотя,
> если используется единожды, то может, и не так страшно.

Приведенный код — это полностью обработчик комманды меню, точнее, TAction. Т.е, он изначально подразумевает интерактивность.

Но, вообще-то, есть мысль реализовать нечто вроде метода, выполняющего предварительную настройку набора без использования интерактивного SetOptions. Пока думаю.

2 Джо © (24.03.06 02:41) [28]

Лично для меня - твой код в [0] очень легко читаем и понятен. Я бы тоже написал бы так.
Более того, я даже не "чураюсь" применять GoTo! Хотя применил его только однажды.
Ну т.е. в "продажной" Дельфи программе. :-)

Я Exit часто использую.

procedure TCalendarInfo.SetPeriod(AStartDate, AEndDate: TDate; ABase: TBaseEmulator; AFormNum: Integer); var i: Integer; begin // Изменение отображаемого периода в календаре if (AStartDate = FStartDate) and (FFormNum = AFormNum) then Exit; // Если период и форма не изменились - выходим FStartDate := AStartDate; // Запоминаем начальную
----------------------------

procedure TMainForm.CalendarGetPaintParams(ACalendar: TPSCCustomCalendar; ADate: TDateTime; var AColor, ABkColor: TColor; var AFontStyle: TFontStyles; ADateType: TPSCCalendarDateType); begin if not Assigned(CalInfo) or (ADateType in [cdtWeek, cdtSelectedWeek]) then Exit; // Если не нужно задавать цвет - выходим if CalInfo.CheckDaysInfo(Calendar.StartDate, CurrentFormNum) then // Если кэш устарел
-----------------------------------

// Выбор формы из списка procedure TMainForm.SelectForm(AItemIndex: Integer); begin if AItemIndex < 0 then Exit;

Джо © (24.03.06 02:38) [27]

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

procedure TMyForm.FooActionExecute (Sender: TObject);
begin
DoFoo; // возможно, с параметрами
end;

var
Ds: IDataSource;
begin
if SelectDataSource(Ds) then
begin
if (not Ds.HasOptions) or (Ds.SetOptions) then
CreateDocFromSource (Ds);
end;
end;

> [31] Игорь Шевченко © (24.03.06 10:25)

Я зачастую и сам стремлюсь так делать, но в этом случае я и не представляю себе название такого метода-уродца, а так хранится себе в acFileRetrievData.OnExecute и все понятно :) Если в дальнейшем замечу дублирование этого кода, то, конечно, что-то придется переиначивать, а пока — пусть будет так.

> [31] Игорь Шевченко

Да, за совет спасибо.

Exit и компания Найти похожие ветки