И снова смех сквозь слезы...

← →
Юрий Зотов © (2010-06-03 07:32) [0]

Надо тут было допилить одну программу. Чужую, неизвестно кем и когда писаную. На Delphi. Дали мне исходники - вперед, парень, разберешься.

Разобрался. Редкий перл.

Во-первых, архитектура программы - ни к черту. Например - используется десяток однотипных форм (что правильно и диктуется задачей), но вместо того, чтобы породить форму-предок и вынести в нее весь общий код (которого было бы немало), этот код повторен в каждой форме. То есть, если что-то меняем, то менять приходится одно и то же 10 раз. С риском где-то что-то упустить, естественно. И с резким возрастанием общего объема кода.

Далее - таких однотипных форм может быть не десяток, а произвольное количество, и работают они последовательно, одна за другой (типа программы-теста) - причем прикладной функционал этих форм может быть самым различным. Архитектура "движок+плагины" ну просто сама напрашивается - и была бы в данном случае более чем уместной. Какое там! И близко нет.

Программа двуязычная. Но для смены языка требует перекомпиляции. Кстати, а как вы думаете, чем переключается язык? Ни за что не угадаете - изменением свойства Tag у одной из кнопок. Если Tag=3, то русский, если 4 - то английский. Причем функционал самой этой кнопки ни с какими языками совершенно не связан.

Для вывода сообщений везде использован MessageDlg. Ну просто классно смотрится, когда вываливается диалог с русским текстом и с английским заголовком.

Ну и на каждом шагу - крутые перлы, например:

Перл 1 (повторяется десятикратно):
try MyForm := TMyForm.Create(...); except MessageDlg("Не хватает памяти", ...); // Прощай, причина ошибки. Exit end; with MyForm do begin Init; DoWork; Destroy; // Именно Destroy, не Free. И здравствуй, возможный мемлик. end;

Перл 2 (повторяется даже на десяти-, а многократно):
try ... finally end; // Именно так, с пустой секцией finally.

Перл 3:
procedure THelpForm.CloseHelp; // public, вызывается во многих местах begin Close end;

И подобных приколов - полно. На каждом шагу. Уж не говорю о том, что при билде выдается порядка сотни хинтов и варнингов, а запуск с включенным Check Range сразу же дает EOutOfRange.

В целом, код даже не грязный - он просто помойка. Что по смыслу, что по оформлению. Вообще, кода юных пионэров довелось видеть немало (увы), но ЭТОТ - просто шедевр. Можно на чемпионат мира выставлять, он бы точно в тройку призеров попал.

Кстати - в числе прочих необходимых доработок заказчик просил устранить такие глюки - при запуске программы иногда:
- или гаснет экран;
- или изображение на экране переворачивается кверху ногами;
- или виснет Windows и потом отказывается загружаться.

Естественно, в коде ничего подобного в явном виде не зашито, к таким эффектам приводит совокупность его глюков. Профи! - к вам обращаюсь! Кто сможет повторить такое, именно в неявном виде? Это вам не хухры-мухры! Розыч - теперь ты понимаешь, что ты по сравнению с этим юным дарованием ты просто птенец, со всеми своими морфиками-полиморфиками-асмами-дебаггерами?

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

Вот такой крик души...

← →
boriskb © (2010-06-03 07:42) [1]

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

Так и есть?
Возможно ли здесь озвучить сколько заплатил заказчик за имеющийся вариант и сколько он собирается платить сейчас?
Пусть не реальные цифры но в сравнении (разы, проценты, порядки)
В назидание, так сказать

← →
Юрий Зотов © (2010-06-03 07:55) [2]

Забыл про еще один прикол, тоже заслуживает публикации:
var MyVar: string; // Глобальная переменная в одном из модулей procedure MyProc; begin MyVar := "Строковая константа"; ... // Пишем MyVar в реестр ... // Тут же читаем MyVar из реестра end;
Больше нигде в программе обращения к реестру нет.

← →
Юрий Зотов © (2010-06-03 08:10) [3]

> boriskb © (03.06.10 07:42) [1]

Сколько заплатил пионэру - не знаю. Опыт подсказывает, что "программер" столь низкой квалификации - это именно пионэр, а пионэрам, как известно, много не платят.

Сейчас заказчик готов платить двухмесячную зарплату неплохого профи в Москве. Не суперпрофи, но достаточно крепкого. Что примерно соответствует истине - потому что программу, по-хорошему, надо делать заново, с нуля - и примерно месяца два на это и уйдет (если, конечно, не не заниматься ничем другим).

← →
Омлет © (2010-06-03 08:32) [4]

А комментарии в коде есть?

← →
Ega23 © (2010-06-03 08:38) [5]

Есть такой сайт http://www.govnokod.ru/
Там и для Delphi секция есть. :)

← →
XXL (2010-06-03 08:51) [6]

Ой! да это же МОЯ программа!
Писал в 98-м когда начал изучать Delphi чтобы чуть заработать после дефолта. Именно так - написана за деньги, но вам Юрий этих денег и на день не хватит...
По поводу зависонов винды не скажу - но переворачивалку экрана и гасилку монитора встраивал в код, подсмотрел на хакерсру :)

← →
Думкин © (2010-06-03 09:02) [7]

А вот у меня было. D серьезнйо дорогущей программе, которую вначале писали европейцы, а потмо микрософтовцы купили и дописывали. И опять же за дорого продавали. Во встроеннйо системе программирования.

a = 1/3+1/3+1/3
b = 1/3+1/3+1/3

c = a+b;

Печатаем с и получаем - 1. Причем так получалось и 10 и 100 и 1000 и т.д.

← →
RWolf © (2010-06-03 09:48) [8]

> Юрий Зотов © (03.06.10 07:32)

with MyForm do begin Init; DoWork; Destroy; // Именно Destroy, не Free. И здравствуй, возможный мемлик. end;

И откуда он возьмётся?
указатель на форму существует только один — внутри with, обнулить его мы не сможем, так что Free или Destroy — без разницы.

← →
Anatoly Podgoretsky © (2010-06-03 09:50) [9]

> Омлет (03.06.2010 08:32:04) [4]

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

← →
12 © (2010-06-03 10:03) [10]

> a = 1/3+1/3+1/3
> b = 1/3+1/3+1/3
>
> c = a+b;
>
> Печатаем с и получаем - 1. Причем так получалось и 10 и
> 100 и 1000 и т.д.
>

ничего не понял

// присваиваем переменной значение 2
сурьезно, причем

Chislo2 : integer;

Chislo2 := Chislo2 + 14; // увеличим на 14

..нафиг chislo2.. нафиг на 14 ..
и так сплошь и рядом.

← →
Anatoly Podgoretsky © (2010-06-03 10:18) [11]

> RWolf (03.06.2010 09:48:08) [8]

Почему не сможем, сможем в Init или в DoWork, из-за ошибок в программе,
фокусы с монитором, показывают что все возможно.

← →
Andy BitOff © (2010-06-03 10:19) [12]

А как вам выдержка из одного faq"а:
error #2046 - возникает из за не верно установленной даты и времени. Будьте добры исправьте её

← →
Юрий Зотов © (2010-06-03 10:20) [13]

> XXL (03.06.10 08:51) [6]
Это вряд ли.
:o)

> RWolf © (03.06.10 09:48) [8]

> И откуда он возьмётся?
Внутри Init или DoWork возникает исключение - и получаем мемлик.

> указатель на форму существует только один — внутри with,
Кто сказал, что MyForm - не глобальная переменная? Хотя в данном случае это и неважно.

> обнулить его мы не сможем
Запросто: MyForm := nil; Даже внутри with. Хотя в данном случае и это тоже неважно.

> так что Free или Destroy — без разницы.
Здесь - да. Но недвусмысленно говорит об аккуратности и зрелости программера. Форматирование кода тоже на работу программы не влияет, однако нормальные программеры его все же форматируют.

==========

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

← →
Ega23 © (2010-06-03 10:23) [14]

> И откуда он возьмётся?

Поднятое исключение в Init или DoWork - и форма осталась висеть в памяти, деструктор не вызвался.
Free или Destroy - ИМХО тут фиолетово, Free просто привычнее.

← →
DVM © (2010-06-03 10:26) [15]

> Думкин © (03.06.10 09:02) [7]

> a = 1/3+1/3+1/3
> b = 1/3+1/3+1/3
>
> c = a+b;

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

← →
RWolf © (2010-06-03 10:32) [16]

> Юрий Зотов © (03.06.10 10:20) [13]
> Кто сказал, что MyForm - не глобальная переменная?
> Запросто: MyForm := nil; Даже внутри with. Хотя в данном случае и это тоже неважно.
Да, верно — я как-то привык такие вещи записывать в форме with TMyForm.Create do try и т.п. и упустил из вида, что форму можно описать и как глобальную переменную )

← →
Думкин © (2010-06-03 10:34) [17]

> DVM © (03.06.10 10:26) [15]

Это было при работе с базой - терялась 1000 уе, все свелось к вот такому.

← →
Игорь Шевченко © (2010-06-03 10:41) [18]

Юрий Зотов © (03.06.10 07:32)

А все к чему - к тому, что программистов мало. Поэтому они стоят денег.

← →
Думкин © (2010-06-03 10:47) [19]

> > Печатаем с и получаем - 1. Причем так получалось и 10
> и
> > 100 и 1000 и т.д.
> >
>
> ничего не понял

Я не удивлен.

← →
Юрий Зотов © (2010-06-03 10:52) [20]

> Омлет © (03.06.10 08:32) [4]
> А комментарии в коде есть?

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

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

Причем комментариев таких появилось около 350. Нормально, да?

← →
@!!ex © (2010-06-03 11:03) [21]

> [20] Юрий Зотов © (03.06.10 10:52)
> Правило у меня есть такое - при доработке чужого кода помечать
> спецкомментарием (с префиксом YZ и датой) все внесенные
> изменения. Чтобы было ясно, кто, когда, что и зачем правил.
> И чтобы искать эти правки легко было.

Diff в системе контроля версий не решает? И кто и когда правил написано...
Зачем засорять код?

← →
Дмитрий Т (2010-06-03 11:09) [22]

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

)))

← →
Palladin © (2010-06-03 11:14) [23]

у хорошего человека это на лбу не написано

← →
Игорь Шевченко © (2010-06-03 11:14) [24]

> Правило у меня есть такое - при доработке чужого кода помечать
> спецкомментарием (с префиксом YZ и датой) все внесенные
> изменения. Чтобы было ясно, кто, когда, что и зачем правил.
> И чтобы искать эти правки легко было.

Вредное правило.

← →
Ega23 © (2010-06-03 11:17) [25]

> Diff в системе контроля версий не решает? И кто и когда
> правил написано...

Ты все ревизии каждого юнита просматриваешь?
Силён, снимаю шляпу.

← →
Игорь Шевченко © (2010-06-03 11:20) [26]

Ega23 © (03.06.10 11:17) [25]

Открой для себя changelog и его создание

← →
Дмитрий Т (2010-06-03 11:24) [27]

По сути скажу.

Думаю, что самым безобразным здесь является это.
try MyForm := TMyForm.Create(...); except MessageDlg("Не хватает памяти", ...); // Прощай, причина ошибки. Exit end;
За это надо жестоко убивать)

Все остальное, безусловно, признак пионерства.

Но упрекнуть его можно в большей степени в отсутствии рефакторинга. Неизвестно же как все писалось? Может возможность обобщения родилась уже в ходе разработки, а изначально форма была одна. Это ты опытный такой, что можешь предвидеть дальнейших ход развития проекта и сразу реализовать что-то с запасом прочности. А он не смог, вернее, не знал, что можно)

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

---

Что-то про совокупностью глюков, приводящих к перевороту, верится с трудом ))
Там должен быть код. Может XXL не врет? )

← →
Anatoly Podgoretsky © (2010-06-03 11:24) [28]

> Внутри Init или DoWork возникает исключение - и получаем
> мемлик.

Но в этом случае замена на Free как мертвому припарка.

> Ega23 (03.06.2010 10:23:14) [14]

А мне привычнее try finally

> DVM (03.06.2010 10:26:15) [15]

C типа Integer

> Игорь Шевченко (03.06.2010 10:41:18) [18]

Особенно Дельфи программисты, это С++ где не плюнь, так в сишника
попадешь!!!

> Юрий Зотов (03.06.2010 10:52:20) [20]

Игрек Зотов :-)

> Дмитрий Т (03.06.2010 11:09:22) [22]

Всяко на хлеб с маслом всегда будет.

> Anatoly Podgoretsky © (03.06.10 11:26) [30]
> > DVM (03.06.2010 10:26:15) [15]
>
> C типа Integer

Нет. Тип был real. Там своя система арифметическая была, поправили в версии 2.4 или 2.5, а мы тогда еще на 2.3 работали.

> Игорь Шевченко (03.06.2010 11:14:24) [24]

Так поступают только трусы

← →
XXL (2010-06-03 11:30) [36]

Вру.
There but for the grace of God, go I

Думаю, что того пионера кинули, обещали заплатить больше а заплатили меньше или не заплатили вообще.
А бета-версию оставили себе, и работают...

← →
Дмитрий Т (2010-06-03 11:30) [37]

> Ega23 © (03.06.10 11:17) [25]
>
>
> > Diff в системе контроля версий не решает? И кто и когда
> > правил написано...
>
>
> Ты все ревизии каждого юнита просматриваешь?
> Силён, снимаю шляпу.

Надо хорошо писать Change Log. Надо входить в роль читателя. Т.е. сделать так, чтобы он понял в общих чертах самое главное в текущем комите - дальше ему поможет уже DIFF.

Например, я когда делаю формальный рефакторинг (например, переименования) я делаю это обязательно отдельным ревижином. В рефижине меняется, допустим, 1000 модулей. Но я честно пишу: "DIFF можно не смотреть, это rename-refactoring такой-то и такой-то".

Бывает, что поменяешь одну строку, а коммент в change log на страницу, т.к. очень важно объяснить в деталях. Или коммент короткий, но есть указание смотреть коммент в коде с помощью DIFF.

Главное добиться того, чтобы у тебя, когда пишешь коммент в Change Log, была уверенность, что ты себя поймешь через год-два, а не будешь говорить - какой урод это писал (вариант, не писал). )))

> Думкин (03.06.2010 11:29:34) [34]

Странно, такое поведение типично при присвоение к Integer

> Дмитрий Т (03.06.10 11:24) [27]

> Может возможность обобщения родилась уже в ходе разработки, а
> изначально форма была одна.

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

> Anatoly Podgoretsky © (03.06.10 11:24) [28]

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

> Anatoly Podgoretsky © (03.06.10 11:32) [38]

Там баг был реальный. Про версии наврал. Исправлено в 3-й версии, 4 или 5-м сервиспаке.

http://www.axforum.info/forums/showthread.php?t=29320

И снова смех сквозь слезы... Найти похожие ветки