| Форумы
ЧАТ |
Орешник |
|
| Форумы -» | Основная («Начинающим»)/ Базы / WinAPI / Компоненты / Сети / Media / Игры / Corba и COM / KOL / FreePascal / .Net / Прочее / RSDN.ORG |
|
| ||
|
|
Новости |
Поиск |
FAQ |
Статьи |
Книги |
Новости VCL
| | Форумы ЧАТ |
Орешник |
|
| |||
| |||
| |||
« предыдущая ветвь | форум | следующая ветвь »
Вложенный цикл [D2005]
Евгений Р. © (2009-07-30 18:05:00)Можно ли из вложенного цикла подать команду break (или continue) для внешнего цикла?
Kolan (2009-08-05 10:48:00) [1]Для данного примера писать все это (а само оно, к сожалению, не генерируется) было очень лень, но в реальном проекте я бы пошел именно этим путем.
Да этот код сложнее изначального, но у него есть приятная особенность — его легко вспоминать. Дело в том, что мой мозг, ничего не может запомнить. Я могу забыть где что в проекте если прервусь на день, а за неделю я забываю вообще все, подчистую. Поэтому пишу так, чтобы, когда я снова буду разбираться в коде, который к этому времени будет как чужой, мне было легко.
Цель того, что я писал в этой ветки — показать как я анализирую и перерабатываю код, а принимать это во внимание или нет — ваше дело.
Еще про комментарии и код у меня есть две заметки:
http://www.ksoftware.ru/wiki/narrative_code
http://www.ksoftware.ru/wiki/hanging_comments
Игорь Шевченко (2009-08-05 01:26:00) [2]
> Ну так это, как я понял, всё генерируется.
Да, вместе с комментариями
palva (2009-08-05 00:34:00) [3]
> И не жалко тебе пальцев ?
Ну так это, как я понял, всё генерируется.
Игорь Шевченко (2009-08-05 00:01:00) [4]Kolan © (04.08.09 23:57) [51]
И не жалко тебе пальцев ? Столько лишнего написать...
Kolan (2009-08-04 23:57:00) [5]О, 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-04 23:38:00) [6]
> А может вместо двух вложенных 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.
palva (2009-08-04 22:46:00) [7]А может вместо двух вложенных for сделать один while с индивидуальным наращиванием ii и jj. На эффективности это вряд ли скажется, зато break будет работать как надо.
Игорь Шевченко (2009-08-04 22:40:00) [8]palva © (04.08.09 22:21) [46]
прошу прощения, break разумеется забыл. Но goto мне здесь кажется излишним.
palva (2009-08-04 22:28:00) [9]Хотя сам алгоритм определения места кристалла тоже уязвим, поскольку кристаллы имеют тенденцию появляться в верхней части фрактальной матрицы. Можно начинать искать со случайного места внутри матрицы, раз уж случайные i и j вычислены и, дойдя до конца продолжать поиск с начала (два воложенных цикла while)
Это замечание к goto не относится.
palva (2009-08-04 22:21:00) [10]
> Игорь Шевченко © (04.08.09 22:08) [45]
Ну здесь отказ от break и уход от попытки прекратить полный просмотр матрицы при первом найденном центре.
Игорь Шевченко (2009-08-04 22:08:00) [11]
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;
Дмитрий Белькевич (2009-08-04 21:53:00) [12]Вообще - не то, что бы я категорически против goto. Но обычно - можно без. Да и label леньво описывать.
Дмитрий Белькевич (2009-08-04 21:50:00) [13]
> и сделать обращение к ней из внутреннего цикла
Как я и говорил, лучше циклы выносить вместе:
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;
palva (2009-08-04 20:43:00) [14]Покритикуйте вот такой код.
Нужно нарисовать кристалл на матрице. Если на матрице уже есть центр кристаллизации (звездочка), то использовать его, если нет, выбрать центр кристалла случайным образом.
{$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 исчезнет. Я бы, наверно, так и сделал. Но не знаю, требуется ли бороться за эффективность при рисовании фрактальных картин.
Kolan (2009-08-04 16:46:00) [15]Про удобство модификации кода из [28] можно поспорить, там есть дублирующийся код, правда чисто теоретически, он слишком маленький, чтобы создать проблемы, поэтому спорить не буду.
Дмитрий Белькевич (2009-08-04 15:41:00) [16]>Это что, goto неявно приведет к таким проверкам? И будет медленнее?
Я что-то не врубился.
Нет, это приведенный в [4] код, с двумя лишними проверками, будет медленнее.
Игорь Шевченко (2009-08-04 15:39:00) [17]
> Считаю код последнего моего примера не менее понятен чем
> код с goto
Считай, твое полное право. Код в [28] является и более понятным и удобным для модификации.
> Может быть Игорь приведет пример где goto был бы весьма
> полезен, может даже из рабочего кода?
Весьма сожалею, но рабочего кода с goto у меня нету по одной простой причине - мне лень объявлять label.
Kolan (2009-08-04 15:03:00) [18]Считаю код последнего моего примера не менее понятен чем код с
goto, но он еще удобнее для модификации.
Может быть Игорь приведет пример где goto был бы весьма полезен, может даже из рабочего кода?
Anatoly Podgoretsky (2009-08-04 15:01:00) [19]> Игорь Шевченко (04.08.2009 14:48:36) [36]
Особенно если поставить целью, ни в коем случае не использовать try
Игорь Шевченко (2009-08-04 14:48:00) [20]goto не так вреден, как его малюют. Практически во всех приведенных примерах стремление к избавлению от goto приводит к запутанности кода.
Оно надо - код запутывать ?
| |
Страницы: 