Форум: "Начинающим";
Текущий архив: 2009.10.11;
Скачать: [xml.tar.bz2];

Вниз

Вложенный цикл   Найти похожие ветки 

← →
Евгений Р.   (2009-07-30 18:05) [0]

Можно ли из вложенного цикла подать команду break (или continue) для внешнего цикла?

---

← →
Ega23 ©   (2009-07-30 18:08) [1]

выставить флаг, сделать Break из текущего, во внешнем проверить значегте флага, если нужно - Break

---

← →
Игорь Шевченко ©   (2009-07-30 18:09) [2]

нельзя

---

← →
Евгений Р.   (2009-07-30 18:09) [3]

Спасибо

---

← →
brother ©   (2009-07-31 07:16) [4]

var _Stop: boolean;
...

_Stop:= False;
repeat
 repeat
   if (срочно_выйти) then _Stop;
 until (...) and (_Stop)
until (...) and (_Stop);

---

← →
brother ©   (2009-07-31 07:17) [5]

>   if (срочно_выйти) then _Stop;

те так:
if (срочно_выйти) then _Stop:= True;

---

← →
RWolf ©   (2009-07-31 09:13) [6]

ещё есть оператор goto, не к ночи будь помянут.

---

← →
Palladin ©   (2009-07-31 09:15) [7]

Abort

---

← →
Плохиш ©   (2009-07-31 11:02) [8]

>  until (...) and (_Stop)
> until (...) and (_Stop);
>

Тогда уж or

---

← →
RWolf ©   (2009-07-31 11:25) [9]

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

---

← →
clickmaker ©   (2009-07-31 15:21) [10]

> if (срочно_выйти) then _Stop:= True;

if (срочно_выйти) then Halt;

---

← →
Б   (2009-07-31 17:32) [11]

> if (срочно_выйти) then Halt;


Это же выход из программы, а не из 2-йного цикла.

1) Проверять булеву переменную.
2) Goto.
3) Метод в методе, в котором нужно вызвать Exit.

---

← →
жж   (2009-08-01 20:45) [12]

> Метод в методе


слон в удаве

---

← →
Б   (2009-08-01 23:35) [13]

> слон в удаве


Почему?
1) Если скорость критична, то это лишний груз.
2) Многие его не любят.

---

← →
brother ©   (2009-08-03 07:38) [14]

> Тогда уж or

ну, да) писАл на скорую руку), но алгоритм понятен)

---

← →
Kolan ©   (2009-08-03 09:10) [15]

Лучше вложенный цикл оформить в виде отдельной функции, вызывать её во внешнем цикле и анализировать её результат.

---

← →
palva ©   (2009-08-03 09:56) [16]

По-моему, если вариант с goto будет проще для понимания и легче читаться, то следует применять goto .

---

← →
brother ©   (2009-08-03 10:48) [17]

по [16] в школе юного бойца не говорили, что использование goto плохой тон?

---

← →
Kolan ©   (2009-08-03 15:18) [18]

palva, вряд ли это будет лучше в данном случае.

---

← →
Ega23 ©   (2009-08-03 15:32) [19]

> по [16] в школе юного бойца не говорили, что использование
> goto плохой тон?


Это миф.

---

← →
palva ©   (2009-08-03 15:47) [20]

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

---

← →
Kolan ©   (2009-08-03 15:51) [21]

Я думаю, palva, что в таком случае goto все сделает еще сложнее. Если алгоритм сложный его лучше прорефакторить использую Извлечение метода.

---

← →
palva ©   (2009-08-03 16:01) [22]

Ну если сделает сложнее, тогда не надо, конечно.

---

← →
Kolan ©   (2009-08-03 16:10) [23]

palva, хотелось бы увидеть пример, когда goto действительно полезен.

---

← →
palva ©   (2009-08-03 19:42) [24]

Я и предлагал goto для решения проблем по сабжу. К примеру, я бы мог написать следующий код:

{$APPTYPE CONSOLE}
type
 TArr = array[1..5,1..5] of Integer;
var
 a: TArr =
 ((1,1,1,1,1),(1,1,1,0,1),(1,1,1,1,1),(1,1,1,1,1),(1,1,1,1,1));
 i, j: Integer;

 procedure FirstZero(const a: TArr; var i,j: Integer);
 var ii, jj: Integer;
 label LBreak;
 begin
   for ii := 1 to 5 do for jj := 1 to 5 do
     if a[ii, jj] = 0 then begin
       i := ii; j := jj;
       goto LBreak;
     end;
   LBreak:
 end;

begin
 FirstZero(a, i, j);
 Writeln(i:3, j:3); //  2  4
end.


---

← →
palva ©   (2009-08-03 21:15) [25]

Когда то я забавлялся паскалем 8000 для IBM 360
http://www.jaymoseley.com/hercules/compilers/contents.htm#PASCAL8000
Исходники компилятора (на Pascal 8000) были просто испещрены подобным применением goto. А что тут удивительного, Pascal 8000 хоть и содержал много любопытных нововведений, (как например оператор forall) но процедуры break не имел. А в компиляторах циклы поиска - это на каждом шагу. А как выходить из цикла, когда найдено? Авторы компилятора не заморачивались введением логической переменной, а выходили через goto. Русское слово "западло" было им неизвестно.

---

← →
Kolan ©   (2009-08-04 10:33) [26]

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


program Project1;

{$APPTYPE CONSOLE}
type
 TMatrix = array[1..5, 1..5] of Integer;

var
 Matrix: TMatrix =
 ((1,1,1,1,1),
  (1,1,1,0,1),
  (1,1,1,1,1),
  (1,1,1,1,1),
  (1,1,1,1,1));

 Row, Col: Integer;

{Найти первый ноль в матрице.
*Если ноль найдется, то его координаты будут помещены в Row, Col}
procedure FindFirstZero(const a: TMatrix; var Row, Col: Integer);

{Найти ноль в указаной строке.
*Функция вернет номер колонки, если найдет ноль
 в указанной строке или -1, если не найдет.}
 function FindFirstZeroInRow(const Matrix: TMatrix; const ARow: Integer): Integer;
 var
   TempCol: Integer;
 begin
   Result := -1;
  {Проверить есть ли строка с указанным номером в матрице}
   if (ARow >= Low(Matrix)) and (ARow <= High(Matrix)) then
   begin
     for TempCol := Low(Matrix[ARow]) to High(Matrix[ARow]) do
       if Matrix[ARow, TempCol] = 0 then
       begin
         Result := TempCol;
         Break;
       end;
   end;
 end;

var
 TempRow: Integer;
 ZeroColIndex: Integer;
begin
 Row := -1;
 Col := -1;
 for TempRow := Low(Matrix) to High(Matrix) do
 begin
  {Поискать ноль в строке.}
   ZeroColIndex := FindFirstZeroInRow(Matrix, TempRow);
  {Если ноль найдется — запомнить его координаты и выйти.}
   if ZeroColIndex <> -1 then
   begin
     Row := TempRow;
     Col := ZeroColIndex;
     Break;
   end;
 end;
end;

begin
FindFirstZero(Matrix, Row, Col);
Writeln(Row:3, Col:3); //  2  4
Readln;
end.


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

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

Еще по поводу примера.
На что бы я обратил ваше внимание, если бы, скажем, вы работали у меня.
1. Название функции не содержит глагола. Хорошее название должно отвечать на вопрос «что сделать?». Ваше название FirstZero оставляет неопределенность, хочется спросить: что первый ноль?

2. Ваш код плохо отформатирован. Кроме мелких придирок, вроде маленьких ii, на которые не стоит обращать внимания, в форматировании есть недостаток — его структура не видна. Мало того, что вы вложили один цикл в другой без begin-end"а (конечно я знаю что так можно, но если вдруг во внешний цикл понадобится что-то добавить, то про begin-end можно забыть и потерять 5-10 минут на поиск проблемы), так еще и вытянули их в одну строку.
Ваш код только кажется проще, на самом деле понять его сложнее.

3. Ну и теперь goto. Ваш пример не годится для того, чтобы показать, что goto бывает полезен, так как в данном примере он совершенно бесполезен, потому что есть Exit. Exit позволить сэкономить две строки.


program Project2;

{$APPTYPE CONSOLE}
type
TArr = array[1..5,1..5] of Integer;
var
a: TArr =
((1,1,1,1,1),(1,1,1,0,1),(1,1,1,1,1),(1,1,1,1,1),(1,1,1,1,1));
i, j: Integer;

procedure FirstZero(const a: TArr; var i,j: Integer);
var ii, jj: Integer;
begin
  for ii := 1 to 5 do for jj := 1 to 5 do
    if a[ii, jj] = 0 then begin
      i := ii; j := jj;
      Exit;
    end;
end;

begin
FirstZero(a, i, j);
Writeln(i:3, j:3); //  2  4
end.


Жду более показательный пример.

---

← →
Дмитрий Белькевич   (2009-08-04 13:08) [27]

> 3. Ну и теперь goto. Ваш пример не годится для того, чтобы
> показать, что goto бывает полезен, так как в данном примере
> он совершенно бесполезен, потому что есть Exit


Полностью согласен. Внешняя процедура (причём именно  с двумя циклами, а не вызов внутреннего цикла из внешнего) + exit.

Плюсы: избавляемся от goto (более структурировано), нет постоянных лишних проверок в цикле (что может замедлить работу раза в 2-3), нет вызова внутреннего цикла из внешнего (в принципе, можно inline цикл сделать, но inline не везде есть, да и ограничения у него есть).

---

← →
palva ©   (2009-08-04 13:48) [28]

С интересом прочитал.
Начну с того, что код для работы я всегда пишу в соответствии с корпоративными стандартами. Здесь же я излагаю исключительно свои представления о прекрасном.

Форматирование в одну строку сделано специально. Можно сказать, что это разворачивание некоего макроса, который сидит у меня в голове. И это макрос называется "просмотреть все элементы матрицы". Аналогично этому я пишу в одну строчку три оператора, обменивающие значения у двух переменных. Для меня это тоже единая смысловая конструкция. В некоторых языках имеется и соответствующая языковая конструкция. Вы ведь должны меня извинить, но паскаль это не тот язык на котором я пишу большую часть моего кода. Где есть оператор обмена переменных, я сейчас язык не припомню, а для просмотра элементов массива можно привести пример на популярном языке C#. Например, если требуется подсчитать количество элементов массива равных трем, то мы пишем:
int[,] a = new int[3,3] {{1,2,3},{1,2,3},{1,2,3}};
int n = 0;
foreach(int i in a) if(i == 3) n++;

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

Вы совершенно правы, что в моем примере можно применить оператор Exit, и пример мой уязвимый. Это, действительно, короче. Но откровенно говоря, goto здесь будет понятней. Плохо когда процедура большая, но по факту она бывает на несколько экранов. И когда тупо смотришь в конец и не понимаешь, почему управление туда не доходит, это не очень здорово. Когда в конце стоит метка, то уже можно поискать выше по тексту, откуда туда делаются переходы. Я всегда стараюсь делать так, чтобы процедура доходила до конца и выходила натыкаясь на end. Если преобразовать алгоритм без потери читабельности не удается, то скрепя сердце пишу exit. Но только потому, что короче. Эквивалентная конструкция с goto ExitProc; понятней.

Но пример все равно неудачный, поскольку можно применить exit. Все что я сейчас пытаюсь придумать взамен, легко сводится без потери ясности к варианту без goto. Может быть, что-нибудь придет в голову, тогда напишу еще. Я как-то не могу припомнить случая, когда мне действительно понадобился goto. Единственный случай, когда я применял goto в рабочем коде это консольные утилиты по обработке текстов. Открывается много файлов, при неудачном открытии одного из них выдается сообщение об ошибке.

{$APPTYPE CONSOLE}
var f1, f2, f3: Text;
label Err;
begin
 Assign(f1, "file1.txt");
 Assign(f2, "file2.txt");
 Assign(f3, "file3.txt");
{$I-}
 Reset(f1);
 if IOResult <> 0 then goto Err;
 Reset(f2);
 if IOResult <> 0 then goto Err;
 Rewrite(f3);
 if IOResult <> 0 then goto Err;
{$I+}
 // to do
 exit;
Err:
 WriteLn("Parameter error");
end.

Здесь можно применить try но в данном случае по понятности это будет эквивалент goto.

---

← →
StriderMan ©   (2009-08-04 13:48) [29]

try
 for ... do
   for ... do
      if SomeCondition then
        Abort;
except
 on E: EAbort do begin end;
 else
   raise;
end;


---

← →
StriderMan ©   (2009-08-04 13:49) [30]

PS: вместо Аборт можно для понятности сделать какой-нибудь свой raise EDoubleBreak.Create;

---

← →
palva ©   (2009-08-04 13:55) [31]

> нет постоянных лишних проверок в цикле (что может замедлить работу раза в 2-3)

Здесь я что-то не понял.

---

← →
Дмитрий Белькевич   (2009-08-04 14:06) [32]

> Здесь я что-то не понял.


Две лишние проверки:


> repeat  repeat    if (срочно_выйти) then _Stop;  until (.
> ..) and (_Stop)until (...) and (_Stop);

---

← →
Дмитрий Белькевич   (2009-08-04 14:09) [33]

В 2-3 раза я, наверно, преувеличил, спорить не буду, но замедление будет.

---

← →
palva ©   (2009-08-04 14:15) [34]

Это что, goto неявно приведет к таким проверкам? И будет медленнее?
Я что-то не врубился.

---

← →
Kolan ©   (2009-08-04 14:46) [35]

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

Вчитываюсь я всегда форматируя код так, как понятно мне (а мне понятно так как в «Стандарт стилевого оформления исходного кода Делфи» http://www.delphikingdom.com/asp/viewitem.asp?catalogid=802), отсюда и замечание.


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


Когда я писал ответ вам, то тоже размышлял о том, где же все таки может пригодится goto, поэтому ваш ответ про большую процедуру ожидал.

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

Дело в том, что большой, тем более на несколько экранов, процедуры не должно существовать. Это типичный запах Длинный метод, goto может служить, как и комментарии, индикатором того, что с методом что-то не так и нужен рефакторинг. А рефакторинг-то как раз неизбежно устранит goto.

Опять таки, в вашем примере с файлами goto можно оставить, но скрипя сердцем, потому что любая модификация повлечет за собой ухудшение ситуации.

Пример

Ваш код я бы переработал так:
program Project2;

{$APPTYPE CONSOLE}

procedure ShowError(const Error: string);
begin
 WriteLn(Error);
 ReadLn;
end;

function OpenFile(const AFileName: string; var Error: string): Boolean;
var
 F: Text;
begin
 Result := True;
 Error := "";

 Assign(F, AFileName);
 {$I-}
 Reset(F);
 if IOResult <> 0 then
 begin
   Result := False;
   Error := "Parameter error";
 end;
 {$I+}
end;

var
 f1, f2, f3: Text;
 Error: string;
begin
 if not OpenFile("file1.txt", Error) then
 begin
   ShowError(Error);
   Exit;
 end;

 if not OpenFile("file2.txt", Error) then
 begin
   ShowError(Error);
   Exit;
 end;

 if not OpenFile("file3.txt", Error) then
 begin
   ShowError(Error);
   Exit;
 end;
end.

А теперь представим, что после открытия файлов нужно сделать еще что-то. И именно в конце. Я очень мало пользуюсь goto, поэтому у меня с трудом  получилось изголится нужным образом:
program Project2;

{$APPTYPE CONSOLE}
var f1, f2, f3: Text;
label Err;
label 1;
begin
Assign(f1, "file1.txt");
Assign(f2, "file2.txt");
Assign(f3, "file3.txt");
{$I-}
Reset(f1);
if IOResult <> 0 then goto Err;
Reset(f2);
if IOResult <> 0 then goto Err;
Rewrite(f3);
if IOResult <> 0 then goto Err;
{$I+}
// to do
goto 1;
Err:
WriteLn("Parameter error");
1:
WriteLn("Job is done."); //Это надо сделать именно тут, не зависимо от ошибок.
Readln;
end.

Получается все более и более запутано. В моем варианте я бы мог просто использовать вложенные условия.


program Project2;

{$APPTYPE CONSOLE}

procedure ShowError(const Error: string);
begin
 WriteLn(Error);
 ReadLn;
end;

function OpenFile(const AFileName: string; var Error: string): Boolean;
var
 F: Text;
begin
 Result := True;
 Error := "";

 Assign(F, AFileName);
 {$I-}
 Reset(F);
 if IOResult <> 0 then
 begin
   Result := False;
   Error := "Parameter error";
 end;
 {$I+}
end;

var
 f1, f2, f3: Text;
 Error: string;
begin
 if not OpenFile("file1.txt", Error) then
   ShowError(Error)
 else
   if not OpenFile("file2.txt", Error) then
     ShowError(Error)
   else
     if not OpenFile("file3.txt", Error) then
       ShowError(Error);

 WriteLn("Job is done."); //Это надо сделать именно тут, не зависимо от ошибок.
 Readln;

end.

Но в моем примере есть еще дублирующийся код — вызов ShowError(Error);, если убрать и его, то будет вообще хорошо.

program Project2;

{$APPTYPE CONSOLE}

procedure ShowError(const Error: string);
begin
 WriteLn(Error);
 ReadLn;
end;

function OpenFile(const AFileName: string; var Error: string): Boolean;
var
 F: Text;
begin
 Result := True;
 Error := "";

 Assign(F, AFileName);
 {$I-}
 Reset(F);
 if IOResult <> 0 then
 begin
   Result := False;
   Error := "Parameter error";
 end;
 {$I+}
end;

var
 f1, f2, f3: Text;
 Error: string;
begin
 if   (not OpenFile("file1.txt", Error))
   or (not OpenFile("file2.txt", Error))
   or (not OpenFile("file3.txt", Error))
 then
   ShowError(Error);

 WriteLn("Job is done."); //Это надо сделать именно тут, не зависимо от ошибок.
 Readln;
end.

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

---

← →
Игорь Шевченко ©   (2009-08-04 14:48) [36]

goto не так вреден, как его малюют. Практически во всех приведенных примерах стремление к избавлению от goto приводит к запутанности кода.

Оно надо - код запутывать ?

---

← →
Anatoly Podgoretsky ©   (2009-08-04 15:01) [37]

> Игорь Шевченко  (04.08.2009 14:48:36)  [36]

Особенно если поставить целью, ни в коем случае не использовать try

---

← →
Kolan ©   (2009-08-04 15:03) [38]

Считаю код последнего моего примера не менее понятен чем код с goto, но он еще удобнее для модификации.

Может быть Игорь приведет пример где goto был бы весьма полезен, может даже из рабочего кода?

---

← →
Игорь Шевченко ©   (2009-08-04 15:39) [39]

> Считаю код последнего моего примера не менее понятен чем
> код с goto


Считай, твое полное право. Код в [28] является и более понятным и удобным для модификации.


> Может быть Игорь приведет пример где goto был бы весьма
> полезен, может даже из рабочего кода?


Весьма сожалею, но рабочего кода с goto у меня нету по одной простой причине - мне лень объявлять label.

---

← →
Дмитрий Белькевич   (2009-08-04 15:41) [40]

>Это что, goto неявно приведет к таким проверкам? И будет медленнее?
Я что-то не врубился.

Нет, это приведенный в [4] код, с двумя лишними проверками, будет медленнее.

---

← →
Kolan ©   (2009-08-04 16:46) [41]

Про удобство модификации кода из [28] можно поспорить, там есть дублирующийся код, правда чисто теоретически, он слишком маленький, чтобы создать проблемы, поэтому спорить не буду.

---

← →
palva ©   (2009-08-04 20:43) [42]

Покритикуйте вот такой код.
Нужно нарисовать кристалл на матрице. Если на матрице уже есть центр кристаллизации (звездочка), то использовать его, если нет, выбрать центр кристалла случайным образом.

{$APPTYPE CONSOLE}
type
 TMatrix = array[0..4,0..4] of Char;
var
 i, j: Integer;
 a: TMatrix = (
  (".", ".", ".", ".", "."),
  (".", ".", ".", ".", "."),
  (".", ".", ".", "*", "."),
  (".", ".", ".", ".", "."),
  (".", ".", ".", ".", ".")
 );

 procedure Crystal(var a: TMatrix);
 var i, j, ii, jj: Integer;
 label LBreak;
 begin
   for ii := 1 to 5 do for jj := 1 to 5 do
     if a[ii, jj] = "*" then begin
       i := ii; j := jj;
       goto LBreak;
     end;
   Randomize;
   i := Random(5); j := Random(5);
   a[i, j] := "*";
LBreak:
   if i > 0 then a[i - 1, j] := "*";
   if i < 4 then a[i + 1, j] := "*";
   if j > 0 then a[i, j - 1] := "*";
   if j < 4 then a[i, j + 1] := "*";
 end;

begin
 Crystal(a);
 for i := 0 to 4 do begin
   for j := 0 to 4 do Write(a[i, j]);
   WriteLn
 end;
end.

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

---

← →
Дмитрий Белькевич   (2009-08-04 21:50) [43]

>  и сделать обращение к ней из внутреннего цикла


Как я и говорил, лучше циклы выносить вместе:


procedure Crystal(var a: TMatrix);
var
 i, j: Integer;
 procedure CrystalInt(var a: TMatrix);
 var
   ii, jj: Integer;
 begin
   for ii := 1 to 5 do
     for jj := 1 to 5 do
       if a[ii, jj] = "*" then
       begin
         i := ii;
         j := jj;
         Exit;
       end;
   Randomize;
   i := Random(5);
   j := Random(5);
   a[i, j] := "*";
 end;
begin
 CrystalInt(a);
 if i > 0 then
   a[i - 1, j] := "*";
 if i < 4 then
   a[i + 1, j] := "*";
 if j > 0 then
   a[i, j - 1] := "*";
 if j < 4 then
   a[i, j + 1] := "*";
end;


---

← →
Дмитрий Белькевич   (2009-08-04 21:53) [44]

Вообще - не то, что бы я категорически против goto. Но обычно - можно без. Да и label леньво описывать.

---

← →
Игорь Шевченко ©   (2009-08-04 22:08) [45]

procedure Crystal(var a: TMatrix);
var i, j, ii, jj: Integer;
begin
  Randomize;
  i := Random(5);
  j := Random(5);
  for ii := 1 to 5 do
    for jj := 1 to 5 do
      if a[ii, jj] = "*" then
      begin
        i := ii;
        j := jj;
     end;
  if a[i, j] <> "*" then
   a[i, j] := "*";
  if i > 0 then a[i - 1, j] := "*";
  if i < 4 then a[i + 1, j] := "*";
  if j > 0 then a[i, j - 1] := "*";
  if j < 4 then a[i, j + 1] := "*";
end;

---

← →
palva ©   (2009-08-04 22:21) [46]

> Игорь Шевченко ©   (04.08.09 22:08) [45]

Ну здесь отказ от break и уход от попытки прекратить полный просмотр матрицы при первом найденном центре.

---

← →
palva ©   (2009-08-04 22:28) [47]

Хотя сам алгоритм определения места кристалла тоже уязвим, поскольку кристаллы имеют тенденцию появляться в верхней части фрактальной матрицы. Можно начинать искать со случайного места внутри матрицы, раз уж случайные i и j вычислены и, дойдя до конца продолжать поиск с начала (два воложенных цикла while)

Это замечание к goto не относится.

---

← →
Игорь Шевченко ©   (2009-08-04 22:40) [48]

palva ©   (04.08.09 22:21) [46]

прошу прощения, break разумеется забыл. Но goto мне здесь кажется излишним.

---

← →
palva ©   (2009-08-04 22:46) [49]

А может вместо двух вложенных for сделать один while с индивидуальным наращиванием ii и jj. На эффективности это вряд ли скажется, зато break будет работать как надо.

---

← →
Игорь Шевченко ©   (2009-08-04 23:38) [50]

> А может вместо двух вложенных for сделать один while с индивидуальным
> наращиванием ii и jj. На эффективности это вряд ли скажется,
>  зато break будет работать как надо.


Не сразу сообразил - ты же из двух циклов выходишь.

Тогда как бы сделал я, чуть больше кода, но мне яснее:

{$APPTYPE CONSOLE}

type
TMatrix = array[0..4,0..4] of Char;
TCrystalCenter = record x, y: Integer end;
var
i, j: Integer;
a: TMatrix = (
 (".", ".", ".", ".", "."),
 (".", ".", ".", ".", "."),
 (".", ".", ".", "*", "."),
 (".", ".", ".", ".", "."),
 (".", ".", ".", ".", ".")
);

function CrystalCenter(M: TMatrix): TCrystalCenter;
var
  I, J: Integer;
begin
  Result.x := -1;
  Result.y := -1;
  for I := 0 to 4 do
    for J := 0 to 4 do
      if M[I, J] = "*" then
      begin
        Result.X := I;
        Result.y := J;
        Exit;
     end;
end;

procedure Crystal(var a: TMatrix);
var
  C: TCrystalCenter;
begin
  C := CrystalCenter(a);
  if (C.X = -1) or (C.Y = -1) then
  begin
    Randomize;
    C.X := Random(5);
    C.Y := Random(5);
    a[C.X, C.Y] := "*";
  end;
  if C.X > 0 then
    a[C.X - 1, C.Y] := "*";
  if C.X < 4 then
    a[C.X + 1, C.Y] := "*";
  if C.Y > 0 then
    a[C.X, C.Y - 1] := "*";
  if C.Y < 4 then
    a[C.X, C.Y + 1] := "*";
end;

begin
Crystal(a);
for i := 0 to 4 do
begin
  for j := 0 to 4 do
    Write(a[i, j]);
  WriteLn
end;
end.

---

← →
Kolan ©   (2009-08-04 23:57) [51]

О, palva, это уже похоже на реальную задачу и вот что я думаю про этот код:

Сначала я, как обычно, ничего не понял, скопировал в Делфи и запустил. Потом стал разбирать код и мне сразу показалось, что тут не хватает комментариев. И я написал их.

procedure Crystal(var a: TMatrix);
var i, j, ii, jj: Integer;
label LBreak;
begin
 {Определить есть ли центр кристализации}
  for ii := 1 to 5 do for jj := 1 to 5 do
    if a[ii, jj] = "*" then begin
      i := ii; j := jj;
      goto LBreak;
    end;
 {Если центра нет, то сделать любую точку центром.}
  Randomize;
  i := Random(5); j := Random(5);
  a[i, j] := "*";
 {Построить кристал.}
LBreak:
  if i > 0 then a[i - 1, j] := "*";
  if i < 4 then a[i + 1, j] := "*";
  if j > 0 then a[i, j - 1] := "*";
  if j < 4 then a[i, j + 1] := "*";
end;

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

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

  Кстати, кажется вы опять назвали вункцию существительным
  или вы имели в виду именно глагол?

Так как тут задача не так абстрактна, то можно сделать хороший рефакторинг. Я решил применить Метод поиска существительных к заданию.

Смотрите: Нужно нарисовать кристалл на матрице. Если на матрице уже есть центр кристаллизации (звездочка), то использовать его, если нет, выбрать центр кристалла случайным образом.

Тут полно существительных:
 о кристалл,
 о матрица,
 о центр кристаллизации.

Все это очень похоже на объекты предметной области. Вот что получилось в итоге:


program Project1;

{$APPTYPE CONSOLE}

uses
 SysUtils,
 Types;

type
{Матрица. На матрице можно построить кристалл.}
 TMatrix = array[0..4, 0..4] of Char;

{Кристал.}
 TCrystal = class
 private
  {Матрица кристала. Без неё кристаллу негде было бы расти.}
   FMatrix: TMatrix;
 protected
  {Найти центр крстализации. Если его нет — будет возвращено (-1, -1)}
   function FindCrystallizationCenter(const Matrix: TMatrix): TPoint;

  {Создать центр кристализации в произвольной точке.
   Возвращает координаты созданного центра.}
   function CreateRandomCrystallizationCenter(var Matrix: TMatrix): TPoint;
 public
  {Кристаллизовать кристалл :)}
   procedure Crystallize;

  {Кристалл просто сохраняет ссылку на переданную матрицу,
   не портите её, пожалуйста.}
   constructor Create(const Matrix: TMatrix);

   property Matrix: TMatrix read FMatrix;
 end;

{ TCrystal }

constructor TCrystal.Create(const Matrix: TMatrix);
begin
 FMatrix := Matrix;
end;

function TCrystal.CreateRandomCrystallizationCenter(var Matrix: TMatrix): TPoint;
begin
 Result.X := Random(Length(Matrix));
 Result.Y := Random(Length(Matrix[Result.X]));
 Matrix[Result.X, Result.Y] := "*";
end;

procedure TCrystal.Crystallize;
var
 {Центр кристализации. Из этой точки возникает кристалл.}
  CrystallizationCenter: TPoint;
begin
{Найти центр кристализации в матрице, вдруг он уже есть.}
 CrystallizationCenter := FindCrystallizationCenter(FMatrix);

{*По-хорошему эта проверка должна быть встроена в TPoint. Точнее вместо TPoint
 нужно сделать свою запись, соответствующую предметной области.}
 if (CrystallizationCenter.X = -1) and (CrystallizationCenter.Y = -1) then
   CrystallizationCenter := CreateRandomCrystallizationCenter(FMatrix);

{Кристализация.}
 if CrystallizationCenter.X > 0 then FMatrix[CrystallizationCenter.X - 1, CrystallizationCenter.Y] := "*";
 if CrystallizationCenter.X < 4 then FMatrix[CrystallizationCenter.X + 1, CrystallizationCenter.Y] := "*";

 if CrystallizationCenter.Y > 0 then FMatrix[CrystallizationCenter.X, CrystallizationCenter.Y - 1] := "*";
 if CrystallizationCenter.Y < 4 then FMatrix[CrystallizationCenter.X, CrystallizationCenter.Y + 1] := "*";
end;

function TCrystal.FindCrystallizationCenter(const Matrix: TMatrix): TPoint;
var
 I, J: Integer;
begin
 Result.X := -1;
 Result.Y := -1;
 for I := Low(Matrix) to High(Matrix) do
   for J := Low(Matrix[J]) to High(Matrix[J]) do
     if Matrix[I, J] = "*" then
     begin
       Result.X := I;
       Result.Y := J;
       Exit;
     end;
end;

procedure WriteLnCrystal(const ACrystal: TCrystal);
var
 I, J: Integer;
begin
 if Assigned(ACrystal) then
 begin
   for I := Low(ACrystal.Matrix) to High(ACrystal.Matrix) do
   begin
     for J := Low(ACrystal.Matrix[J]) to High(ACrystal.Matrix[J]) do
       Write(ACrystal.Matrix[I, J]);
     WriteLn;
   end;
 end;
end;

var
 Crystal: TCrystal;

 Matrix: TMatrix = (
 (".", ".", ".", ".", "."),
 (".", ".", ".", ".", "."),
 (".", ".", ".", "*", "."),
 (".", ".", ".", ".", "."),
 (".", ".", ".", ".", ".")
);

begin
 try
   ReportMemoryLeaksOnShutdown := True;

   Crystal := TCrystal.Create(Matrix);
   try
     Crystal.Crystallize;
     WriteLnCrystal(Crystal);
     ReadLn;
   finally
     Crystal.Free;
   end;

 except
   on E:Exception do
     Writeln(E.Classname, ": ", E.Message);
 end;
end.



Вместо вашей функции появился почти осязаемы кристалл, который можно кристаллизовать.

Конечно в коде полно недостатков. Матрица — статический массив и передаётся в класс по ссылке, хорошо бы сделать массив динамическим, и копировать его внутри.

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

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

P. S. У меня уже глаза слипаются, если где-то наделал ошибок — простите, я просто показывал идею.

Кстати, можно продолжить. Некоторые методы вроде поиска и выставления случайного центра кристаллизации попахивают «Feature Envy», так что из лучше перенести использую Перенесение метода в другой класс, которого сейчас вообще нет, в Матрицу. Может завтра руки дойдут.

---

← →
Игорь Шевченко ©   (2009-08-05 00:01) [52]

Kolan ©   (04.08.09 23:57) [51]

И не жалко тебе пальцев ? Столько лишнего написать...

---

← →
palva ©   (2009-08-05 00:34) [53]

> И не жалко тебе пальцев ?

Ну так это, как я понял, всё генерируется.

---

← →
Игорь Шевченко ©   (2009-08-05 01:26) [54]

> Ну так это, как я понял, всё генерируется.


Да, вместе с комментариями

---

← →
Kolan ©   (2009-08-05 10:48) [55]

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

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

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

Еще про комментарии и код у меня есть две заметки:
http://www.ksoftware.ru/wiki/narrative_code
http://www.ksoftware.ru/wiki/hanging_comments

---

Страницы: 1 2 вся ветка

Форум: "Начинающим";
Текущий архив: 2009.10.11;
Скачать: [xml.tar.bz2];

Наверх

Память: 0.66 MB
Время: 0.007 c