Ода молодому поколению

← →
Kerk © (2011-09-07 11:19) [80]

Скорость понимания кода при чтении такой подход снижает в разы.
Попробуй разгляди разницу между cn, pn и nn, не вчитываясь в каждую букву.

← →
Kerk © (2011-09-07 11:21) [81]

Да и попробуй догадайся сразу чего вы там зашифровали в этих аббревиатурах.

← →
Компромисс (2011-09-07 11:28) [82]

> В понятности ничего не добавляют... Чего плодить-то
>

1. Добавляют и еще как. Например, мне в коде Sha пришлось голову напрячь, чтобы понять суть алгоритма
2. Опечаток меньше. Трудно вместо next набрать current. Да и легче ошибку в чужом коде найти, если вдруг current := nil присваивается. А на p := nil никто не отреагирует
3. Ctrl+пробел позволяет выбрать переменную из списка, а не набирать ее имя вручную. Поэтому между q и next нет большой разницы. Проблемы будут (и с восприятием в том числе), есть иметь переменные nodeNext, nodeCurrent и nodePrevious, ну так я поэтому и написал именно nextNode, а не наоборот

← →
Игорь Шевченко © (2011-09-07 11:33) [83]

> Да и легче ошибку в чужом коде найти, если вдруг current
> := nil присваивается. А на p := nil никто не отреагирует

Поддерживаю целиком и полностью

← →
CRLF (2011-09-07 11:35) [84]

Нам на первом курсе списки читали как раз с переменными p и q, по Вирту... %-)

← →
Игорь Шевченко © (2011-09-07 11:37) [85]

CRLF (07.09.11 11:35) [84]

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

← →
Kerk © (2011-09-07 11:41) [86]

Во, люди понимают :)

← →
TUser © (2011-09-07 11:48) [87]

Надо называть все переменные в стиле iCounterOfElements, так всякому сразу понятно, что это целое число. А дочитав до конца название, - даже понятно, зачем оно. Такой правильный стиль называется обфускацией.

function FindElementInListGivenAsParameter (ListWhichWeSentToFindFunction: TListOfHZFoo; ElementWhichWeLookForInListGivenAsFirstParameterOfThisFunction: THZFoo); var iCounterOfElementsInListGivenAsParameter: integer; begin for iCounterOfElementsInListGivenAsParameter := 0 to ListWhichWeSentToFindFunction.CountOfElementsInList - 1 do if ListWhichWeSentToFindFunction.ElementsOfList[i] = ElementWhichWeLookForInListGivenAsFirstParameterOfThisFunction then begin result := i; exit; end; result := -1; end;

← →
Kerk © (2011-09-07 11:50) [88]

> TUser © (07.09.11 11:48) [87]

Это называется доведением до абсурда - демагогический прием.

← →
Sha © (2011-09-07 11:53) [89]

> Компромисс (07.09.11 10:12) [73]

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

> q должно было быть next (или nextNode),

По смыслу алгоритма q - временная переменная, используемая для ротации значений, как здесь:
temp:=a; a:=b; b:=temp;
Зачем называть ее nextNode, если тут идет работа с двумя списками,
и Next"ов тут 2, вернее он тут 1, но одновременно он же Prev в другом списке,
ну, в общем, ты понял.

> p должно было быть current (или currentNode),

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

> вместо Result должна была быть еще одна переменная previos (previousNode),

Какой, нафиг, previos, если это голова нового списка

> а Result присваиваться только один раз (Result := Current).

Не взлетит. Где бы ты не написал такое, это будет ошибкой.
В начале работы у тебя Current - голова старого списка,
в конце - nil.

> Лично я убил бы за имена переменных.

Я бы тоже. А за ошибку - еще раз.

← →
Kerk © (2011-09-07 11:55) [90]

> Sha © (07.09.11 11:53) [89]

Ну вот видишь, ты настолько плохо подобрал имена, что Компромисс с простейшим алгоритмом сходу не смог разобраться. Отличный пример.

← →
TUser © (2011-09-07 12:00) [91]

> Это называется доведением до абсурда - демагогический прием.

Зато легко доходит.

Человек способен прочесть 5-15 строк примитивного кода, который является единым блоков (процедурой или методом) то есть воспринимается как единое целое. Замена там i: integer на абсурд затрудняет погнимание, а не облегчает его. Длинно должны называться те переменные, для которых разница между хотя бы парочкой мест их использования (включая объявление) больше этих 15 строк. Также как и те переменные, которые используются в разных единых блоках (процедурах или методах). То есть:

в коде длинной функции (а лучше, чтобы таких функций вовсе не было),

либо

поля классов, параметры процедур и методов, глобальные переменные

.

← →
Kerk © (2011-09-07 12:02) [92]

> TUser © (07.09.11 12:00) [91]

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

← →
Компромисс (2011-09-07 12:08) [93]

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

Вполне возможно. Но не понял я именно из-за плохих имен, то есть по твоей вине :-)

← →
Компромисс (2011-09-07 12:12) [94]

> Выше отличный пример. У Sha он получился даже лучше, чем
> я думал. Код написан в стиле "знаю алгоритм, поэтому понимаю
> код", а должно быть наоборот "вижу код, понимаю алгоритм".
>

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

← →
Sha © (2011-09-07 12:15) [95]

это надо сильно принять, чтобы не увидеть обмен трех значений.

← →
Kerk © (2011-09-07 12:17) [96]

> Sha © (07.09.11 12:15) [95]

Причем здесь сам обмен трех значений?

1) Для чего он нужен?
2) ЧТО он обменивает?

← →
Sha © (2011-09-07 12:26) [97]

> 1) Для чего он нужен?

У функции говорящее название

> 2) ЧТО он обменивает?

Первая итерация покажет.

А если не помогло, см. [89] :-)

← →
Kerk © (2011-09-07 12:29) [98]

> Sha © (07.09.11 12:26) [97]

Ну т.е., судя по ссылке на [89], ты вместо того, чтоб дать переменным понятные имена, предлагаешь написать пояснительный комментарий в 3 раза длиннее функции?

← →
Компромисс (2011-09-07 12:40) [99]

function Reverse(list: PNode): pNode; var previous, next, current: pNode; begin; current := list; previous:=nil; while current<>nil do begin; next:=current.Next; current.Next:=previous; previous:=current; current:=next; end; Result := previous; end;

Кстати, в моем варианте Result = nil, если параметр nil.

← →
Компромисс (2011-09-07 12:42) [100]

Кстати, еще убил бы и за то, , что в одной строке несколько операторов.

← →
Inovet © (2011-09-07 12:49) [101]

> [100] Компромисс (07.09.11 12:42)
> Кстати, еще убил бы и за то, , что в одной строке несколько операторов.

Так здесь обмен через temp т.е. одно логическое действие, почему бы и не в одной строке. Убивать за другое надо.

← →
Компромисс (2011-09-07 12:54) [102]

Inovet © (07.09.11 12:49) [101]
Так здесь обмен через temp т.е. одно логическое действие, почему бы и не в одной строке. Убивать за другое надо.

Посмотрите [99]. Я всего лишь переименовал переменные в цикле. Нет там никакого обмена через temp. Создавалась лишь видимость обмена из-за неправильных имен переменных.

← →
Компромисс (2011-09-07 12:56) [103]

Кстати, next (q) лишняя.

function Reverse(list: PNode): pNode;
var
previous, next, current: pNode;
begin;
current := list;
previous:=nil;
while current<>nil do begin;
current.Next:=previous;
previous:=current;
current:=current.Next;
end;
Result := previous;
end;

← →
Компромисс (2011-09-07 12:57) [104]

В [103] ошибся.

← →
Sha © (2011-09-07 13:36) [105]

Все-таки, есть где ошибиться.
И это точно имена )

← →
Павел Калугин © (2011-09-07 13:50) [106]

> pasha_golub © (05.09.11 10:57) [3]
> Кстати, озвучилось мнение, что Сапер для студента - это
> слишком сложно. Кто согласен?

Народ на втром-третьем куррсе на турбопаскале изваял "супертетрис" на двоих игроков и ниче. А тут сапер слишком сложно.. Ужос..

← →
Romkin © (2011-09-07 13:58) [107]

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

← →
Павел Калугин © (2011-09-07 14:03) [108]

Romkin © (07.09.11 13:58) [107]
> Думаете многие студенты это напишут, хотя бы за час-другой?

Из тех, с кем я учился, процентов 80..

Второй пример: те же условия, только на объектах и с рекурсией.

← →
Компромисс (2011-09-07 14:19) [110]

И испытания, разумеется, на обращение пустого списка, списка из одного элемента, из нескольких.

Понемногу подводить к Test Driven Development? :)

← →
Компромисс (2011-09-07 14:20) [111]

Второй пример: те же условия, только на объектах и с рекурсией.

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

Romkin © (07.09.11 14:03) [109]

Наверно, так
function TNode.Reverse: TNode; function Renew(New, Old: TNode): TNode; var Tail: TNode; begin; if Old<>nil then begin; Tail:=Old.GetNext; Old.SetNext(New); Result:=Renew(Old, Tail); end else Result:=New; end; begin; Result:=nil; Renew(Result, Self); end;

Последний оператор заменить на
Result:=Renew(Result, Self);

или лучще оба на :-)
Result:=Renew(nil, Self);
Так вроде приятнее смотрится

> Из тех, с кем я учился, процентов 80..

А из тех, что приходят, процентов 80 - нет :(

← →
Компромисс (2011-09-07 15:18) [116]

Sha

Надо метод Renew переименовать. Только не знаю, как :-) Разбираться в коде лень, а что он делает, из названия непонятно.

> Sha © (07.09.11 15:05) [112]

Приставить процедуру в объект - это не совсем ООП :)
При переходе к объектам важно отойти от процедурного подхода:
TList = class Key: Word; Tail: TList; ...
И у Tail как объекта есть те же самые методы, которые можно вызвать, он же тоже TList.

> Компромисс (07.09.11 15:18) [116]
> Надо метод Renew переименовать

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

> Romkin © (07.09.11 15:28) [117]

Можно совсем в ООП стиле написать
че-нить вроде MoveStackToStack(Source, Dest),
но в сочетании с требованием рекурсии ужасно смотреться будет.
И так уже - ух, а так - ваще будет )

Самый первый процедурный вариант гораздо понятнее.
ООП - не самоцель.

> Компромисс (07.09.11 12:40) [99]
>
> function Reverse(list: PNode): pNode;
> var
> previous, next, current: pNode;
> begin;
> current := list;
> previous:=nil;
> while current<>nil do begin;
> next:=current.Next;
> current.Next:=previous;
> previous:=current;
> current:=next;
> end;
> Result := previous;
> end;

А чего у тебя в коде единобразия нетути? :) То отделяется оператор присваивания пробелами, то нет. Почему begin висит на строчке с условием, да еще и с пустым оператором после него?

Ай-айай, а еще боремся за звание дома высокой культуры и быта (с)

Вообщем, по результам ветки готов взять Sha © к себе студентом. Не знаю, научу ли программировать, но сало с водкой употреблять возможно :)

Ода молодому поколению Найти похожие ветки