Оцените код.

← →
@!!ex © (2007-11-22 18:05) [40]

> перестраховываешься? :) И уж очень много условий. Тогда
> форматровать красивее надо.

Не перестраховка, скорее всего менял код и не углядел, что проверка уже есть...
Смысл от перестраховки такой? :)

> Хотя if Dist<MaxDistance then begin сам так делаю. Но
> некоторым не нравится. :)

Что плохо?

> Эти и еще некоторые методы не выполнятся если Dead=true.
> Так может ставить проверку в начале if dead then exit; ?

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

> Некоторые Draw настолько похожи. Почему не запихнуть общее
> в функцию. Или Draw сделать у базового класса TGameObject
> и этим кодом?

думал над этим, но придумать не мог. Спасибо этой ветке и комментаторам(и тебе лично, твой пост и помог) идея появилась.
Как раньше до нее не додумался - х.з.
Наверно это потому, что в ВУЗе учили не использовать exit...

← →
vpbar © (2007-11-22 18:08) [41]

> Model:=MdlLib.GetModel(Path+"Data\Models\Ships\"+FileName+"\Main.
> otm");
> constructor TGun.Create(const Pos, Dir: TVector; GunModel:
> PModel; GunSide:integer);
> ........
> Model:=GunModel;
> ....
> while Model^.

Так уверены что нигде в Create не будет nil в GunModel. А MdlLib.GetModel может вернуть nil? Вот тут я бы перестраховался. Хотябы асертами.

← →
@!!ex © (2007-11-22 18:11) [42]

> Так уверены что нигде в Create не будет nil в GunModel.
> А MdlLib.GetModel может вернуть nil? Вот тут я бы перестраховался.
> Хотябы асертами.

Не. Там nil быть не может.
Это контейнер моделей.
если модель загружена ранее - он просто вернет указатель на нее.
Если же модель не загружена, то попытаеться загрузить, если это не получиьтся, то будет ошибка на уровне контейнера, нет смысла перестраховываться.

← →
vpbar © (2007-11-22 18:13) [43]

while Model^.GetMeshID("Fire_begin_"+IntToStr(I))>-1 do begin inc(FireCount); SetLength(Fires,FireCount); Fires[FireCount-1].effect:=TLinearEmitter.Create(effect_ShootFire,1); Fires[FireCount-1].PBegin:=Model^.GetMeshID("Fire_begin_"+IntToStr(I)); Fires[FireCount-1].PEnd:=Model^.GetMeshID("Fire_end_"+IntToStr(I)); Model^.SetMeshVisible(Fires[FireCount-1].PBegin,False); Model^.SetMeshVisible(Fires[FireCount-1].PEnd,False); inc(I); end; Fires последовательно увеличивается SetLength -гов. От этого проблемы бывают. FireCount-1 = LastFire. Или вобще сделай контейнер для Fire вместо Fires.

← →
Черный Шаман (2007-11-22 18:16) [44]

> @!!ex © (22.11.07 17:39) [20]
>
> Кстати. еще вопрос, наличие Destroy обяхательно?
> Где-то недалеко отсюда слышал, что Destroy должен быть всегда,
> даже если он пуст

Просто хорошее правило, так как часто требуется что-то добавить в него.

← →
vpbar © (2007-11-22 18:16) [45]

> > Хотя if Dist<MaxDistance then begin сам так делаю. Но
>
> > некоторым не нравится. :)
>
> Что плохо?

Незнаю. Говорят не по штандарту

>>@!!ex © (22.11.07 18:11) [42]
Тогда да. Можно. Хотя я все равно бы асерт вставил. :)

← →
vpbar © (2007-11-22 18:37) [46]

if ((DistanceToTarget>1000) and (Action<>GOA_MOVE)) or ((DistanceToTarget>100) and (Action=GOA_MOVE)) then Direction.Y:=Direction.Y-AI.GetAngle(Position,Target.Position,GetTVector(pi/16*FrameTime,0,0),GetTVector(Direction.Y/180*pi,0,0)).X/pi*180 else Direction.Y:=Direction.Y-AI.GetAngle(Position,Target.Position,GetTVector(pi/16*FrameTime,0,0),GetTVector((Direction.Y-90)/180*pi,0,0)).X/pi*180
жуть да? Тогда поясню на коротком примере

В таких случаях
if flag then DoCall(a/b,a*c) else DoCall(a/b,a*k);
Я делаю так.
if flag then parametr:=a*c; else parametr:=a*k; DoCall(a/b,parametr);
Почему. Во-первых потому что мне не нравится, что в первом варианте компилятор не оптимизирует вызов. А во-вторых потому что яснее видно различие в вызовах. Это может и не совсем правильно. Да и в делфи приходится лазить вверх и придумывать имя переменно. Но все же, почему то мне так больше нравится.
ЗЫ
Прокоментируйте, пожалуйста, насколько так не правильно делать.

← →
clickmaker © (2007-11-22 18:40) [47]

> в первом варианте компилятор не оптимизирует вызов

то есть? что он там должен соптимизировать?

← →
turbouser © (2007-11-22 18:43) [48]

if (Sleep=0) and Я бы не стал слипом обзывать.

← →
vpbar © (2007-11-22 18:44) [49]

//clickmaker © (22.11.07 18:40) [47]
кладем на стек (регистры) параметры в зависимости от условия делаем вызов. А он в лоб.
cmp jne @@else одинаковый код маленькое отличие call jmp @@endif @@else: одинаковый код маленькое отличие call @@endif:
Хотя можно так
cmp одинаковый код jne @@else маленькое отличие jmp @@endif @@else: маленькое отличие @@endif: call

← →
vpbar © (2007-11-22 18:46) [50]

vpbar © (22.11.07 18:44) [49]
простите промахнулся. Так во втором случае:
одинаковый код cmp jne @@else маленькое отличие jmp @@endif @@else: маленькое отличие @@endif: call

← →
@!!ex © (2007-11-22 18:46) [51]

> [48] turbouser © (22.11.07 18:43)

А как?
Это используется, Когда объект уже мертв(тоесть никаких действий производить не должен, не трисовываеться и вообще), но эффекты еще не кончились.
Например ракета долетела до цели,взорвалась, эффект взрыва привязан к ракете. если ракету удалить, то взрыв не воспроизведется. Поэтому ракета еще живет секунд 5, пока эффект проигрывается, но сама уже никаких действий не выполняет. Спит...

← →
vpbar © (2007-11-22 18:48) [52]

>>@!!ex © (22.11.07 18:46) [51]
Просто sleep процедура есть с таким именем. Поэтому перекрывать не рекомендуют.

← →
@!!ex © (2007-11-22 18:50) [53]

> [50] vpbar © (22.11.07 18:46)

Кстати, насчет оптимизации...
А разве лишнее значение(flag) в стэк класть не будет в сумме работать столько же времени, сколько и вариант без флага?

← →
turbouser © (2007-11-22 18:50) [54]

> @!!ex © (22.11.07 18:46) [51]

Вариантов - тьма
например IsSleep Sleeping Active Deactivated InUse Dream :-)

← →
vpbar © (2007-11-22 18:54) [55]

> !!ex © (22.11.07 18:50) [53]

Оптимизация не во флаге. Вместо него можно все что возвращает логический тип. Просто писать условие ((DistanceToTarget>1000) and (Action<>GOA_MOVE)) or ((DistanceToTarget>100) and (Action=GOA_MOVE)) я посчитал слишком длинным и ненужным

← →
vpbar © (2007-11-22 18:55) [56]

>>@!!ex © (22.11.07 18:46) [51]
Т.е. sleep - это метод? Или флаг. Если метод то обзывать глаголом - нормально в принципе. А если флаг, то странно.

← →
@!!ex © (2007-11-22 18:55) [57]

> [55] vpbar © (22.11.07 18:54)

Понял. ступил не туда посмотрел

← →
vpbar © (2007-11-22 18:55) [58]

← →
@!!ex © (2007-11-22 18:56) [59]

> [58] vpbar © (22.11.07 18:55)

Это вообще счетчик. :)

← →
vpbar © (2007-11-22 19:07) [60]

> vpbar © (22.11.07 18:55) [58]

Тогда тоже странно.

← →
Kolan © (2007-11-22 19:23) [61]

> Как то не очень скрыто получилось:
> if AI.FIsAimedOnTarget and (LastShoot<=0) and (AI.FDistance<400)
> and (Action=GOA_ATTACK) then begin

Я не пойму о чем речь, если ве не понимаете что такое инкапсуляция, то это тема отдельной ветки.

А вот буквы F  этот хорошо код вроде:
AI.FIsAimedOnTarget можно добавить к списку «запахов» файлура. Он так и кричит караул, нарушена инкапсуляция!

> if not Dead then  лучше не инвертировать.

Тут не согласен, надо так, как в данном конкретном случае лучьше.

> Просто хорошее правило, так как часто требуется что-то добавить
> в него.

Имхо лишнего не должно быть  зачем?

И вообще, ветка уходит не туда.
Одно дело научить автора верно оформлфть код на Delphi  это не сложно. (Буковки, пробельчики)
Час на изучения правил, неделя на запоминание. Проверено на себе, я же тоже раньше писал как курица лапой.

А другое дело научить программировать/ООП видеть «bad smells»  это совсем другое. Это долго&#133

Предлагаю ограничится форматирование.

> Тогда тоже странно.

Тогда страшно :)

← →
Anatoly Podgoretsky © (2007-11-22 19:24) [62]

> @!!ex (22.11.2007 18:56:59) [59]

Но если это счетчик, то почему не SleepCounter и вопроса не будет, а так приходится переспрашивать.

← →
Anatoly Podgoretsky © (2007-11-22 19:25) [63]

> @!!ex (22.11.2007 18:56:59) [59]

Кстати ты уже понял, как бы твой код оценили.

← →
@!!ex © (2007-11-22 19:28) [64]

> Я не пойму о чем речь, если ве не понимаете что такое инкапсуляция,
> то это тема отдельной ветки.
>
> А вот буквы F — этот хорошо код вроде:
> AI.FIsAimedOnTarget можно добавить к списку «запахов» файлура.
> Он так и кричит караул, нарушена инкапсуляция!

Я так не пишу.
Меня интересует, почему скрытые методы доступны таким макаром?
Написал чисто проверить, а он взял и откомпилился...
Зачем?

> Предлагаю ограничится форматирование.

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

> [63] Anatoly Podgoretsky © (22.11.07 19:25)

Да, уже понял. :)
Поэтому и спросил, чего хря позориться? :))

← →
vpbar © (2007-11-22 19:31) [65]

>>@!!ex © (22.11.07 19:28) [64]
Это "фишка" дефи. Приватные поля видны в пределах модуля. Делаен ненужным друзей. Хотя может задумывалось не для этого :)

← →
Kolan © (2007-11-22 19:34) [66]

> А вот если вы объясните как лучше с точки зрения ООП писать
>  буду очень благодарен.

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

Простите про рефакторинг(видимо еще не прочли «Рефакторинг. Улучшение существующего кода.» еще лучьше на английском.
Если не знаете паттеронов(«Паттерны проектирования»), то хотябы про простые рефакторинги и плохие запахи  польза будет

Все три книги есть тут:
http://www.ksoftware.ru/library.html

← →
Kolan © (2007-11-22 19:36) [67]

> Это «фишка» дефи. Приватные поля видны в пределах модуля.
> Делаен ненужным друзей. Хотя может задумывалось не для этого
> :)

Имхо, это гадость Delphi(версии семь и ниже) на которую я часто попадался. Начинаешь класс из модуля копировать, а он обращается к плолям других классов просто по невнимательности. Слава богу в BDS уже есть strict.

← →
@!!ex © (2007-11-22 19:41) [68]

> [66] Kolan © (22.11.07 19:34)

"Паттерны проектирования" я читал.
А вот насчет "Рефакторинг. Улучшение существующего кода." - эт нет, спасибо.

> @!!ex (22.11.2007 19:28:04) [64]

> почему скрытые методы доступны таким макаром

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

> эт нет, спасибо.

Хм&#133 Почему? Можеш еще про GRASP паттерны прочесть. Крег Ларман&#133

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

Бегло глянул - что сразу не понравилось:
1. нестандартное форматирование
2. пустые конструкторы/деструкторы обектов (только вызов inherited)
3. нежелательно работать с булевыми переменными, т.е. конструкции плана if a > b then C := True;

остальное глядеть лениво :)

нежелательно = нежелание :)

> Но было бы интересно, как с точки зрения правильного программирования
> должен выглядеть код(если отбросить форматирование).

Пойми форматирование  это основа, без него тебя не пойму.

> Но было бы интересно

А просто так пересказывать тебе книги я не хочу. Вот ты когда их прочтёш, и задашь такой вопрос:

«
вот у меня есть метод
procedure TCraftGAS1.Update(dt: integer);
Я чувствую что тут пахнет Long Method"ом, но как применить рекамендованый в этом случае рефакторинг
Extract Method я ума не приложу
»

От тогда я тебе конкретно отвечу, если смогу :)&#133

> Посылать я этот код уже врядли буду, чего позорить?

Посылай  это твой уровень чего стеснятся, не возьмут так не возьмут, ты же не сможешь за день два разобраться в толпе книг и методик&#133

> без него тебя не поймут.

> 3. нежелательно работать с булевыми переменными, т.е. конструкции
> плана if a > b then C := True;

имеешь ввиду
C:=a>b; ?
ПРивычка старая, не делать так. Потому что значение C:=true строго определенное. в C помойму -1 лежит.
А если использовать C:=a>b то там может лежать что угодно... Накололся так однажды не хило...

> @!!ex © (22.11.07 20:00) [72]

В иделае - код должен быть максимально компактным, универсальным и при этом
удобочитаемым и понятным :-)

> Пойми форматирование — это основа, без него тебя не пойму.

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

> ты же не сможешь за день два разобраться в толпе книг и
> методик…

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

еще сильно напрягает то, что я смотрю на код, который писали профи у которых я учился... и там не все хорошо, если судить по тому, что говорили в этой ветке...

Оцените код. Найти похожие ветки