Оцените код.

← →
@!!ex © (2007-11-22 17:13) [0]

Предложили попробоваться в весьма не кислую фирму.
Но для того, чтобы туда назначили собеседование, надо отослать им пример личного кода.
Я знаю, что у меня есть проблемы в плане качества кода.
Вот и хочеться услашыть мнение мастеров, чем плох этот код(кроме отсутствия комментов):
http://afera-net.narod.ru/AITypes.rar
Этот код реализует поведения всякой нечисти в мире.

← →
KSergey © (2007-11-22 17:17) [1]

Так вы в фирму-то и шлите.
Они и оценят. Что не понравится - если удасться - можно будет у них же и узнать.

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

Так не интересно. :)
Предпочту узнать заранее, что в нем не так. ;)

← →
Celades © (2007-11-22 17:19) [3]

СРазу бросилось в глаза: обилие магических цифр.

← →
Судья (2007-11-22 17:19) [4]

> Предпочту узнать заранее, что в нем не так. ;)

Это нечестно перед работадателем.

← →
Правильный_Вася (2007-11-22 17:19) [5]

> код реализует поведения всякой нечисти в мире.

предлагаешь расплодить ее у нас на компах?

← →
@!!ex © (2007-11-22 17:20) [6]

> Это нечестно перед работадателем.

Ок. уговорили. :)

← →
vpbar © (2007-11-22 17:20) [7]

покритиковать. щааас. пока настроение есть.

← →
Черный Шаман (2007-11-22 17:21) [8]

> @!!ex © (22.11.07 17:13)
>
> Предложили попробоваться в весьма не кислую фирму.
> Но для того, чтобы туда назначили собеседование, надо отослать
> им пример личного кода.
> Я знаю, что у меня есть проблемы в плане качества кода.
> Вот и хочеться услашыть мнение мастеров, чем плох этот код(кроме
> отсутствия комментов):
> http://afera-net.narod.ru/AITypes.rar
> Этот код реализует поведения всякой нечисти в мире.

Паттерн Паблик-Морозов? Хотя для начинающего (1 год в серьезном программировании) нормально.

← →
@!!ex © (2007-11-22 17:23) [9]

> [8] Черный Шаман (22.11.07 17:21)

То что все в public"e лежит - это не ошибка.
Так нужно для дебага.

← →
Reindeer Moss Eater © (2007-11-22 17:29) [10]

ни одного гоуту, ни одного виз"а. некрасиво как-то.

← →
Kolan © (2007-11-22 17:29) [11]

1. Function  запомни все жирное с маленькой(кроме некоторых исключений типа Message в Exception)
2. Отвратный прием выравнимания по имени.
Constructor Create; Function GetAngle
3. LookAtTarget:boolean;
Boolean пишется с большой. И где пробел после «:»? Поля надо назаватьс буквы F.
Правильно так: FLookAtTarget: Boolean;
Кроме того что поля делают в public?

Далее у тебя поле названо глаголом Look а глаголы  это методы. Поля  это существительные. Оч. грубая ошибка.
Далее в коде их много&#133

4. AI_GoAway
Стиль с и с++ в Delphi недопустим  это я про подчеркивание.

5. Position,Target,MaxAngle, Angle
Между MaxAngle, Angle есть пробел а остальные кто будет ставить?

Правильно так: Position, Target, MaxAngle, Angle

6. Function GetAngle(const Position,Target,MaxAngle, Angle:TVector):TVector; override;
Заступ за черту 80  тоже минус, надо было перенести:
function GetAngle(const Position, Target, MaxAngle, Angle: TVector): TVector; override;
Опятьже после «:» надо ставить пробел всегла, а до него никогда. Ессно к оператору присвоения не относится.

7. effect_explode:TEmitter;
Вообще пипец. Почему стиль изменился? Почему он стал си образным.

8. single; integer;
Все это с большой. Все что не жирное с большой :)

9. TBuildingStatic
public
protected
Поменяны местами  плохо.

10. Реализацию не смотрел. Если исправишь выше описаное посмотрю.
LookAtTarget:=abs(AngleBetween)<pi/15; Result.X:=0;
Опятьже пробелов не хватает. Abs, Pi c маленькой
Надо:
LookAtTarget := Abs(AngleBetween) < Pi/15; Result.X := 0;

Общее впечатление  плохое в основном из-за толпы не контролируемых public и published полей.
И непонятных названий, особенно глаголы в полях&#133

← →
@!!ex © (2007-11-22 17:32) [12]

> Правильно так: FLookAtTarget: Boolean;
> Кроме того что поля делают в public?

Писать отдельную обертку типа:
property IsOnTarget:Boolean read FLookAtTarget write FLookAtTarget;
А смысл?

← →
Kolan © (2007-11-22 17:33) [13]

Увидел еще:
procedure TDestroyer.Update(dt: integer);

Это вообще из вредных советов, грубая ошибка, очень. Что за dt? Почему не е001 или hte?

← →
@!!ex © (2007-11-22 17:34) [14]

> [13] Kolan © (22.11.07 17:33)

DelayTime.. это даже не моя идея.
Сколько видел проектов больших игр, демок, везде dt....

← →
Kolan © (2007-11-22 17:35) [15]

> А смысл?

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

LookAtTarget

Объясни что это значит по русски, я предложу имя для переменной.

← →
@!!ex © (2007-11-22 17:37) [16]

Типа так:
TAIIntercept = class(TAI)
protected
FIsGoAway:Boolean;
FIsLookAtTarget:Boolean;
FDistance:Single;
public
Constructor Create;
Function GetAngle(const Position,Target,MaxAngle, Angle:TVector):TVector; override;
property IsGoAway:Boolean read FIsGoAway;
property IsLookAtTarget:Boolean read FIsLookAtTarget;
property DistanceToTarget:Single read FDistance;
end;

← →
Kolan © (2007-11-22 17:37) [17]

> DelayTime&#133 это даже не моя идея.

А вот я не видел игр, но знаю что dt  это дэтэ и не чего более. А DelayTime  время задержки.

Чтобы писать нормальный код надо использовать только 1(одни) прием. &#133 Надо писать код не для себя, а для другого программиста, который его будет читать не зависимо от того, случится это или нет&#133

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

> Это много говорит о тебе. Смысл в инкапсуляции и тем что
> ты ей не пользуешься. Уверен что если посидеть, то можно
> найти толпу мест где поля должны быть доступны только для
> чтения, но ты это контролируешь у себя в голове а не в коде…
> А голова у тебя такая одна и если я сяду за твой ког я
> могу все испротить…

Я уже понял. Согласен на 100%.

← →
Riply © (2007-11-22 17:38) [19]

> [11] Kolan © (22.11.07 17:29)
> 4. AI_GoAway
> Стиль с и с++ в Delphi недопустим — это я про подчеркивание.

"А мужики то и не знают" и пишут: ListView_SetItemCount :)

← →
@!!ex © (2007-11-22 17:39) [20]

Кстати. еще вопрос, наличие Destroy обяхательно?
Где-то недалеко отсюда слышал, что Destroy должен быть всегда, даже если он пуст

← →
Kolan © (2007-11-22 17:39) [21]

> property IsGoAway:Boolean read FIsGoAway;
> property IsLookAtTarget:Boolean read FIsLookAtTarget;
>
> property DistanceToTarget:Single read FDistance;

Я незнаю смысл может эти поля и должны быть доступны для чтения/записи, может нужны сеттеры геттеры, это по итуации, но одно из простых rule of thumb  все поля приватные! Еснно можно отступать от этого, но ввиде исключения, а у тебя наоборот&#133

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

> [19] Riply © (22.11.07 17:38)

Кстати, причина того, что я там поставил подчервикание - чтобы отделить AI от GoAway.
Но сама приставка AI тут явно лишняя, поэтому и уточнять не стал, исправил просто.

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

> [21] Kolan © (22.11.07 17:39)

Так эта.. я не понял, правиьно или нет?

← →
Kolan © (2007-11-22 17:41) [24]

> «А мужики то и не знают» и пишут: ListView_SetItemCount
> :)

Вот такие дела&#133 подчеркивание  это стиль си, который предложили, &#133 как там их Керниган и Ричи(кажется).
А на Delphi пишут используя CamelCase. Кроме того CamelCase стандарт UML. И в Jave принят тот же стиль(кроме геттеров и может еще чего я не спец), и в C#.

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

Почему выравнивание по имени - это плохо?
Вроде так проще читаеться..

← →
Kolan © (2007-11-22 17:43) [26]

> Так эта&#133 я не понял, правиьно или нет?

Если тебе не надо давать коиенту писать в поле, то не давай тут нет правильно/неправильно, тут еть как надо, а для ответа на этот вопрос мне надо разобраться в коде, понять что кто делает&#133

А вот названия могу предложить, если опишите что это значит на русском.

← →
Riply © (2007-11-22 17:44) [27]

> [24] Kolan © (22.11.07 17:41)

Ничего не могу возразить, кроме того, что привыкла ориентироваться на "генофонд".
Раз там используют - значит можно :)

← →
Kolan © (2007-11-22 17:44) [28]

> Почему выравнивание по имени  это плохо?
> Вроде так проще читаеться&#133

Это не проше, это не правиьно, это не соотв обепринятому стилю VCL http://www.citforum.ru/programming/delphi/style_delphi/.

← →
Kolan © (2007-11-22 17:47) [29]

Раз там используют Очень редко посмотрите. Подчеркивание использется для служебных целей:
_AddRef напимер специально с подчеркивания, чтобы было ясно что его трогать не надо(compiler automatically generates a call to _AddRef).

← →
@!!ex © (2007-11-22 17:47) [30]

TAIIntercept = class(TAI) protected FIsGoAway:Boolean; FIsLookAtTarget:Boolean; FDistance:Single; public Constructor Create; Function GetAngle(const Position,Target,MaxAngle, Angle:TVector):TVector; override; property IsGoAway:Boolean read FIsGoAway; property IsLookAtTarget:Boolean read FIsLookAtTarget; property DistanceToTarget:Single read FDistance; end;
Классик с помощью метода GetAngle расчитывает направление движения объекта. Например, поворачивает истребитель на цель.
IsGoAway - отвечает на вопрос, заходит истребитель на цель, или наоборот летит от нее, чтобы отлетев на достаточное расстояние - развернуться на следующий заход.
IsLookAtTarget - указывает, нацелен ли сейчас объект на цель или же нет. Нацелен - это значит что вектор по направленю к цели, и вектор движения объекта меньше заданного.
DistanceToTarget - расстояние до цели.

← →
Reindeer Moss Eater © (2007-11-22 17:49) [31]

IsAimedOnTarget

← →
@!!ex © (2007-11-22 17:49) [32]

> 7. effect_explode:TEmitter;
> Вообще пипец. Почему стиль изменился? Почему он стал си
> образным.

Это не си образный стиль.
Это отделение префикса.
поскольку есть префиксы, например:
effect
texture
model
то было решено, чтобы проще оринетироваться - писать их с маленькой буквы и отделять подчеркиванием.

P.S>
Совместимость с VCL значения не имеет, он здесь не используется.

← →
Kolan © (2007-11-22 17:50) [33]

> Типа так:

Нет типа так:

TAIIntercept = class(TAI) private FIsGoAway: Boolean; FIsLookAtTarget: Boolean; FDistance: Single; public constructor Create; //большая буква function GetAngle(const Position, Target, MaxAngle, Angle: TVector): TVector; override; // Перенос, пробелы, большая буква property IsGoAway: Boolean read FIsGoAway; // Пробелы property IsLookAtTarget: Boolean read FIsLookAtTarget; // Пробелы property DistanceToTarget: Single read FDistance; // Пробелы end;

И названия исправить.

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

Почему private?

← →
Kolan © (2007-11-22 17:53) [35]

> Совместимость с VCL значения не имеет, он здесь не используется.

Тогда вам на сайт того, на чем вы пишите.

Я могу прокоментить только то, как принято писать на Delphi я это стиль VCL.

Сравните
[31] IsAimedOnTarget

И ваше

LookAtTarget

Я понял первое как «Нацелена на цель» Да/Нет, второе «посмотреть/посмотри на цель»

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

Это исеть чистый си стиль. все с маленькой а слова отделяются «_»

← →
@!!ex © (2007-11-22 17:54) [36]

> Сравните
> [31] IsAimedOnTarget
>
> И ваше
>
> LookAtTarget

Ок.

← →
Kolan © (2007-11-22 17:55) [37]

> Почему private?

Пашта они скрыты должны быть, пашта инкапсуляция&#133

← →
@!!ex © (2007-11-22 17:58) [38]

> [37] Kolan © (22.11.07 17:55)

Как то не очень скрыто получилось:
if AI.FIsAimedOnTarget and (LastShoot<=0) and (AI.FDistance<400) and (Action=GOA_ATTACK) then begin
Вполне успешно компилится, хотя вызывается из куска кода, который никакого отношения к TAIIntercept не имеет...

← →
vpbar © (2007-11-22 17:59) [39]

по поводу форматирования. - есть непривычности. Но DelForEXP все исправляет. Так что это мелочи, имхо.
Хотя if Dist<MaxDistance then begin сам так делаю. Но некоторым не нравится. :)
...
if not Dead then - лучше не инвертировать. Т.е. сначала ветка true, потом false. В данном случае может лучше не Dead а Аlive?
if (Sleep=0) and (not Dead) then begin if not Dead and WorldMap.Objects.ObjectCorrect(Target) and (Target.Life>0) and (target.Model<>nil) then begin - перестраховываешься? :) И уж очень много условий. Тогда форматровать красивее надо.

> procedure TBomb.Draw;
> procedure TBomb.Update(dt: integer);
Эти и еще некоторые методы не выполнятся если Dead=true. Так может ставить проверку в начале if dead then exit; ?
Некоторые Draw настолько похожи. Почему не запихнуть общее в функцию. Или Draw сделать у базового класса TGameObject и этим кодом?

← →
@!!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)

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

← →
Anatoly Podgoretsky © (2007-11-22 19:42) [69]

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

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

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

← →
Kolan © (2007-11-22 19:45) [70]

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

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

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

> [70] Kolan © (22.11.07 19:45)

Там опечатка. Спасибо! Я качаю книжку уже.

← →
@!!ex © (2007-11-22 20:00) [72]

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

← →
Rouse_ © (2007-11-22 20:05) [73]

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

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

← →
Rouse_ © (2007-11-22 20:05) [74]

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

← →
Kolan © (2007-11-22 20:08) [75]

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

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

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

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

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

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

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

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

← →
Kolan © (2007-11-22 20:09) [76]

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

← →
@!!ex © (2007-11-22 20:10) [77]

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

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

← →
turbouser © (2007-11-22 20:12) [78]

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

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

← →
@!!ex © (2007-11-22 20:12) [79]

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

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

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

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

← →
@!!ex © (2007-11-22 20:13) [80]

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

← →
Kolan © (2007-11-22 20:15) [81]

> Главное чтобы он был опеределен.

Я те говорю он обределен  это VCL. Если трудно понять его, то есть выделеное описание этого формата.
Писать не по стандарту это как не чистить зубы  не умрешь, но всем вокруг противно.

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

dt, кстати, используют..

← →
Anatoly Podgoretsky © (2007-11-22 20:19) [83]

> @!!ex (22.11.2007 20:10:17) [77]

> if a > b then C := True;

Так нельзя

> C:=a>b;

Это не взаимозаменяемые конструкции, мне правда не понятна зачем нужна эта конструкция, может где ни будь делается C := False иначе это одноразовый выключатель.
Без всего кода и его разборки не понять, но что то странное.
А код как сказали "тебя не поймут".

← →
vpbar © (2007-11-22 20:20) [84]

>>@!!ex © (22.11.07 20:10) [77]

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

???!!! В C лежит или True или False. Остальное от лукаваго. Это ж тебе не Си.
Наколешься конечно если с логическими переменными будешь пытаться делать то к чему они не предназначены.

← →
@!!ex © (2007-11-22 20:21) [85]

http://afera-net.narod.ru/src.rar

← →
@!!ex © (2007-11-22 20:21) [86]

> [84] vpbar © (22.11.07 20:20)

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

← →
Anatoly Podgoretsky © (2007-11-22 20:26) [87]

> @!!ex (22.11.2007 20:12:19) [79]

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

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

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

← →
vpbar © (2007-11-22 20:26) [88]

>>@!!ex © (22.11.07 20:21) [86]
один бит. это как? В смысле при чем тут работа с boolean как с целым числом?

← →
Anatoly Podgoretsky © (2007-11-22 20:27) [89]

> @!!ex (22.11.2007 20:13:20) [80]

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

← →
Anatoly Podgoretsky © (2007-11-22 20:29) [90]

> @!!ex (22.11.2007 20:18:22) [82]

Но не как DelayTime, а как локальная переменная DataTime.
Ведь тебя не зря спросили, что такое dt - значит это вызывает вопрос.

← →
Anatoly Podgoretsky © (2007-11-22 20:31) [91]

> vpbar (22.11.2007 20:20:24) [84]

Вот в его конструкции с if как раз черт знает что лежит, может вообще неопределенное значение.
Но конструкции не равны из-за if ветви то else нет.
Это известная логическая ловушка, применение конечно возможно, как стоп-флаг.

← →
@!!ex © (2007-11-22 20:33) [92]

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

Да не смешение стилей. У нас в фирме стандарт такой:
стандартные типа с маленькой быквы пишутся.
привефиксы пишутся с маленькой буквы и отделяются подчеркиванием.
обычные перменные пишутся с большой буквы и в качестве разделителя тоже большая буква.
Смешение стилей - это когда:
GetBigValue
get_small_value

> путаница с глаголами и существительными.

Это есть, постараюсь исправиться.

> Плюс непонимание ООП в части скрытие полей, их контроль
> через свойства.

понимаю. уже достаточно объяснили.
просто ООП обучали на С++, там не property.

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

Меня только один момент волнует по данному вопросу.
Если есть поле Value и к нему внешний доступ полный должен быть.
Зачем делать обертку property?

> один бит. это как? В смысле при чем тут работа с boolean
> как с целым числом?

Надо было сохранить boolean значение в 1 бит данных в файле.
Я исходил из того, что там либо 0, либо -1, в итоге пролетел нехило, как раз из-за присваения переменной булевских выражений.

← →
vpbar © (2007-11-22 20:34) [93]

>>Anatoly Podgoretsky © (22.11.07 20:31) [91]
Дык я и не говорил что конструкции одинаковые. Я по этому поводу
> А если использовать C:=a>b то там может лежать что угодно.

возмущался.

← →
@!!ex © (2007-11-22 20:35) [94]

> [90] Anatoly Podgoretsky © (22.11.07 20:29)

Не. как раз народ у которого я код смотрел dt была везде и значила она delaytime.
Т.к. в играх всеже считается исяходя из проемужтка времени прошедшего с последнего расчета, то dt - везде.
наверно поэтому и такое сокращение, если везде писать DelayTime, очень много замусорится.

← →
vpbar © (2007-11-22 20:41) [95]

> Меня только один момент волнует по данному вопросу.
> Если есть поле Value и к нему внешний доступ полный должен
> быть.
> Зачем делать обертку property?

Незачем. Если только на будущее.

> > один бит. это как? В смысле при чем тут работа с boolean
>
> > как с целым числом?
>
> Надо было сохранить boolean значение в 1 бит данных в файле.
>
> Я исходил из того, что там либо 0, либо -1, в итоге пролетел
> нехило, как раз из-за присваения переменной булевских выражений.

:) Это наследие Си. В делфи, по-моему, не гарантируется что там -1. "A value of type ByteBool, LongBool, or WordBool is considered True when its ordinality is nonzero. " Цитата из справки

← →
Anatoly Podgoretsky © (2007-11-22 20:43) [96]

> @!!ex (22.11.2007 20:33:32) [92]

> Меня только один момент волнует по данному вопросу.
> Если есть поле Value и к нему внешний доступ полный должен быть.
> Зачем делать обертку property?

Сравни

public FVar: type
и

private F: type procedure Setter(F: type); public/published property Var1: FVar read FVar write FVar; property Var2: FVar read FVar write Setter; property Var2: FVar read FVar;

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

1. только косвенный, контролируемый компилятором.
2. только косвенный на чтение и контролируемый запись.
3. только косвенный на чтение, доступ по записи запрещен.

А доступ выглядит одинаково, весь механизм в других модуляд скрыт от конечного пользователя, а в модуле можно обращаться как к Var, так и к F

V := Var;

Это очень мощный механизм и он не ограничен приведеными примерами, синтаксис шире.

← →
vpbar © (2007-11-22 20:47) [97]

>>Anatoly Podgoretsky © (22.11.07 20:43) [96]
Так вы за наших или за марсиан :).
Если нужен прямой доступ для чтение-записи то чем property Var1: FVar read FVar write FVar; лучше public FVar: type ?
По-моему ничем. Все одинакова, ну кроме извращений с получением адресов.

← →
Anatoly Podgoretsky © (2007-11-22 20:48) [98]

> vpbar (22.11.2007 20:34:33) [93]

Если это Boolean, а по присвоению именно он, то там не может лежать ничего кроме True/False, это же не Boolean := Bool

← →
vpbar © (2007-11-22 20:48) [99]

>>Anatoly Podgoretsky © (22.11.07 20:43) [96]
Так вы за наших или за марсиан :).
Если нужен прямой доступ для чтение-записи то чем property Var1: FVar read FVar write FVar; лучше public FVar: type ?
По-моему ничем. Все одинаково, ну кроме извращений с получением адресов и published.

← →
@!!ex © (2007-11-22 20:49) [100]

> [96] Anatoly Podgoretsky © (22.11.07 20:43)

А какая разница косвенный доступ или прямой?

Ну в любом случае мне не избежать переменных в паблике.
потому что есть места куда надо сохранять указатели на переменные и с ними работать...
А тут никак через property не реализовать...

← →
vpbar © (2007-11-22 20:51) [101]

>>@!!ex © (22.11.07 20:49) [100]
Ууу. Указатели на поля. Жуть. Аккуратнее.

> А тут никак через property не реализовать

Да. Это надо просто по другому решать.

← →
Anatoly Podgoretsky © (2007-11-22 20:51) [102]

> @!!ex (22.11.2007 20:35:34) [94]

Для меня подобно сокращение значит, или дельта t, или DateTime - но никогда не DelayTime
А касательно насчет "мусорить", то программист не имеет права на лень. Если будет написано DelayTime, то вопросов нет, а если dt то это вопрос.

Ты читал http://www.podgoretsky.com/DM/BadTips.html совет 0?
Это про тебя и для тебя.

← →
Anatoly Podgoretsky © (2007-11-22 20:54) [103]

> vpbar (22.11.2007 20:41:35) [95]

> Если есть поле Value и к нему внешний доступ полный должен
> быть.
> Зачем делать обертку property?

> Незачем. Если только на будущее.

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

← →
Anatoly Podgoretsky © (2007-11-22 20:56) [104]

> vpbar (22.11.2007 20:47:37) [97]

> Так вы за наших или за марсиан :).

Я уже объяснил, чем лучше.
Так что я за культурных марсиан.

← →
@!!ex © (2007-11-22 20:58) [105]

> [101] vpbar © (22.11.07 20:51)

Знаю что жуть. Но другие способы очень медленные.
Скрипт должен работать с переменными(например жизнью игрока).
ему либо косвенно задавать переменную(что влечет за собой несколько вызовов всевозможных методов).
Либо просто работать со своими внутренними переменынми, среди которых зарегена и жизнь игрока.
Что не влечет за собой ниодного вызова.

> [102] Anatoly Podgoretsky © (22.11.07 20:51)

Читал.
dt - единственное сокращение во всем коде.
и, кстати, я таки ошибся, кодеры действительно имели ввиду dt - как дельта тайм. :)
переводы строк есть, там где это удобнее.
Например в больших блоках if.
Комментариев нету... Это да. На это тупо нет времени, комментарии писать. Да и никгде не учат писать комменты, даже в сорсах VCL их минимум.
Юнитов у меня много. около 50 штук. А лишние пложить ох как нехочеться(если вы намекаете на то, что все ИИ в одном юните собраны), ИМХо им в одном юните самое место.
Так что из совета 0 про меня только отсутствие большого количчества комментов, и единственная переменная сокращенная.

> @!!ex (22.11.2007 20:49:40) [100]

Разница простая, ты не можешь передать адрес и над ним извращенно издеваться. А скажем завтра тебе потребовалось при присвоение делать проверки, так достаточно будет дописать private метод и переписать определение свойства и продолжать работать. Без этого тебе возможно придется переворошить кучу кода заменить где встретится присвоение на вызов функции/процедуры, ничего не пропустить.
Свойство для того и придумали, чтобы уменьшить расходы на программирование и повысить надежность программы, а кое кому укоротить ручки по непредусмотренному использованию полей, в обход проверок и диких манипуляций с адресами.

> @!!ex (22.11.2007 20:58:45) [105]

> Например в больших блоках if.
> Комментариев нету... Это да. На это тупо нет времени, комментарии писать.

Лень не приветствуется.
При нормальном имени комментарий не требуется.
Делай выводы.

>>Anatoly Podgoretsky © (22.11.07 20:56) [104]
Понял я понял. Собственно я согласен. Сам делаю обертки полей всегда. Автоматически так сказать.

> @!!ex (22.11.2007 20:58:45) [105]

> Так что из совета 0 про меня только отсутствие большого количчества комментов, и единственная переменная сокращенная.

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

> Лень не приветствуется.

Дело не в лени. А в отсутствии времени.

> При нормальном имени комментарий не требуется.

Дайте материала по правильному названию методов и переменных?

> @!!ex (22.11.2007 21:06:50) [110]

Правило номер один - не сокращайте до 1/2 символов, если это не является общепринятым, в твоем случае это не так, поскольку несколько человек спросили, что это такое.

Правило номер два - никогда не выставляйте подобный код на всеобщее обозрение.

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

Да чего ж вы так все привязались к единственному сокращению, к тому же для данной сферы стандартному. :)
в проекте около 700 раз используется сочетание dt...
код вообще не читаемый будет, если писать delaytime или deltatime, потому что dt - вездею ниодно действие без него не происходит.

я http://www.ozon.ru/context/detail/id/3159814/ читал. Полезно было и есть.

> Правило номер один - не сокращайте до 1/2 символов, если
> это не является общепринятым, в твоем случае это не так,
> поскольку несколько человек спросили, что это такое.

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

> Правило номер два - никогда не выставляйте подобный код
> на всеобщее обозрение.

Почему? Наоборот вроде очень полезно!

>>@!!ex © (22.11.07 21:10) [112]
Согласен. К тому там где связано со временем dt вполне логично. t -время. d - дельта( ну греческая букавка такая. Ее замена.) dt- приращение времени.

> @!!ex (22.11.2007 21:10:52) [112]

Можно согласиться со всем, кроме читаемости.
Похоже ты сам себя уговариваешь.

> @!!ex (22.11.2007 21:12:54) [114]

> Видимо те кто спрашивал - никогд ане писали приложений, которые синхронизируются по времени.

Ошибаешься.
Но никогда в голову не приходило так сокращать, в угоду лени.

Вообще, проблема в том, что нет возможности в редакторе вставить нормальные символы. Такие как ΔΣ≤≥∆≠∏

> [118] Anatoly Podgoretsky © (22.11.07 21:17)

Нет. Че мне себя уговаривать?
Тем боее что называть переменные как a,b,c,d,..,aa,ab,ac,...,ba,bb, нету.
dt - удобнее и вполне читабельно.

> Вообще, проблема в том, что нет возможности в редакторе
> вставить нормальные символы. Такие как ???????

Неее. не надо такого счастья. не удобно было бы.

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

> Тем боее что называть переменные как a,b,c,d,..,aa,ab,ac,
> ...,ba,bb, нету.

нету привычки

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

Учитывая что почти все конторы занимающиеся графикой в 3Д пишут на С++, им мое форматирование по любому не понравится.
Так что это не так актуально. :)

> Прочтите про рефакторинг(видимо еще не прочли «Рефакторинг.
> Улучшение существующего кода.» еще лучьше на английском.

Все на Яве... Очень сложно воспринимать. :(

> Все на Яве&#133 Очень сложно воспринимать.

Да это так кажется. Основа там вообше текстом описана + UML, а понимать Джаву всеравно надо, так как большинство OO bigots"ов(как они себя называют) используют Джаву для примеров&#133

> [128] Kolan © (23.11.07 09:34)

читаю минут 15, уже несколько идей как разбить методы на части и избавиться от дублирования кода... спасибо! отличная вещь! хоть примеры все равно плохо воспринимаю

> читаю минут 15,

Прочтите до третей главы(Chapter 3. Bad Smells in Code) включительно. Запахи надо помнить иначе можешь пропустить, а исправление их  дело технику уже&#133

И помните рефакторить код ради рефакторинга ненадо, рефакторьте когда добавляете новые функциональности, когда исправляете ошибку&#133 Ну там написано когода&#133

Пц... если бы мой код разбирали на собеседованиях так же как тут, работал бы я щас на рынке Юность приемщиком бутылок, и горя не знал бы )

Оставлю форматирование и имена переменных на усмотрение разработчика. В конце концов "на глазок" всё терпимо )

Вопрос, код критичен по скорости? Если да, то я нашел несколько моментов к которым докопался бы )

В частности:
1. Попробуй переписать TGun.Create представив себе что у PModel есть функция GetMeshCount.
2. Попробуй обойтись меньшим количеством Round-ов в TGun.Shoot.
Новые варианты сюда запости.

Кстати, ще заметил тут следы некоторых глобальных переменных, хотя их объявления не нашел. Очевидно они все в юните Data. Было бы интересно узнать сколько там глобальных переменных )

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

И еще, написать на скорую руку можно и:
Field: TFieldType
а уже потом, когда вдруг резко изменятся требования к коду и понадобится осуществлять контроль шаловливых ручек наших братьев по разуму - индусов, тогда можно и наростить обертку property... Конечно если на это (подготовку кода к... агрессивной среде) будет выделено дополнительное время/деньги.

Мое резюме: код интуитивно понятен, есть неоптимальные моменты (которые возможно будут исправлены в следующих постах), нет привычки работать в команде, нет коментариев.
А оценка уже зависит от того кто конторе нужен )

> Anatoly Podgoretsky © (22.11.07 20:51) [102]

"The site www.podgoretsky.com was blocked, it is in the Restricted Personal Homepages category"

эх :(

> Kerk (23.11.2007 10:25:13) [133]

Не нравлюсь я вам. А может podgoretsky.com понравится или по ИП

> 1. Попробуй переписать TGun.Create представив себе что у
> PModel есть функция GetMeshCount.

GetMeshCount - там есть. Но он возвращает общее количчество мешей, а мне нужны именно конкретные, и их количество нигде не указано к сожалению. и указать - возможности нет...

> 2. Попробуй обойтись меньшим количеством Round-ов в TGun.Shoot.

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

> Кстати, ще заметил тут следы некоторых глобальных переменных,
> хотя их объявления не нашел. Очевидно они все в юните Data.
> Было бы интересно узнать сколько там глобальных переменных
> )

Много. Но там в основном контейнеры. Например, текстурные, модельные. вобщем те вещи, которые должны быть реализованы через singleton, но тогда я еще не знал что глобальные переменные - это плохо, и что есть такая вещь, как singleton.
собственно модуль Data и был создан для того, чтобы хранить там переменные, которые должны существовать в единичном экзмепляре и должны быть доступны везде.
А сейчас если переделывать, придеться все переписывать.
В ледующем проекте так и собираюсь поступить, а текущий придеться оставлять как есть.

> нет привычки работать в команде, нет коментариев.

Да, уже два кода как работаю в гордом одиночестве...
А комментарии.. никто никогд ан учил их ставить. Даже когда в команде работал.

> А оценка уже зависит от того кто конторе нужен )

Мда.. эникейщие из меня получится замечательный.

>singleton
Эх... как говорила одна знакомая: "илитарная выдумка" )
Хотя конечно поупражнять мозги этим можно. Даже потом можно красивыми словами поразмахивать.

>А комментарии.. никто никогд ан учил их ставить. Даже когда в команде работал.
Знач фиговая команда... Или не команда вовсе, а просто потом сидели толпой и собирали куски вместе, попутно объясняя друг другу детали интерфейсов и реализации. Коментарии, как это ни прискорбно, нужны.

>Мда.. эникейщие из меня получится замечательный.
Смелее ) "Лучше сожалеть о сделаном, чем о не сделаном." (це) непомню.
Ты главное объясни, что ты хочешь учиться работать в команде, повышать качество и отказоустойчивость своего кода, изучать новые технологии, придумывать/находить и использовать новые идеи... Думать головой и этим приносить прибыль себе и компании.

> Знач фиговая команда... Или не команда вовсе, а просто потом
> сидели толпой и собирали куски вместе, попутно объясняя
> друг другу детали интерфейсов и реализации. Коментарии,
> как это ни прискорбно, нужны.

25 челевок(было когда я там работал, сейчас не знаю).
за 10 лет три успешных проекта, каждый бюджетом на несколько миллионов долларов.
один проект точно получил минимум 7 наград. Это почти все журналы отечественные + КРИ.
Да и не было проблем с пониманием кода. В свое время влился в проект на 4 метра исходников примерно за месяц, почти при полном отсутствии комментариев.

С тех пор и не особо воспринимаю необходимость комментариев.

> >singleton
> Эх... как говорила одна знакомая: "илитарная выдумка" )
> Хотя конечно поупражнять мозги этим можно. Даже потом можно
> красивыми словами поразмахивать.

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

← →
_xxx_ (2007-11-23 12:00) [139]

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);

Имхо, зачем делать inc(FireCount) в начале, когда лучше сделать это в конце? Не придется 5 раз повторять FireCount-1, лишняя трата ресурсов.

> [139] _xxx_ (23.11.07 12:00)

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

← →
_xxx_ (2007-11-23 12:12) [141]

Хм. Интересно, а в той конторе когда твой код проверять будут, догадаются ли они сделать следующее или нет?:
http://www.yandex.ru/yandsearch?clid=9582&text=SetMeshVisible

:)

> [141] _xxx_ (23.11.07 12:12)

Этот код я им посылать не буду. :)
Сказал уже. :))

← →
_xxx_ (2007-11-23 12:16) [143]

>>_xxx_ (23.11.07 12:00) [139]
Каких ресурсов? Если вы о скорости, то об этом пусть компилятор заботится. А если сделать так как вы сказали, то логика немного теряется, по-моему. Хотя этот кусок кода вообже нехороший.

> >singleton
> Эх&#133 как говорила одна знакомая: «илитарная выдумка» )

Ты нас недалеких просвети, что это значит-то&#133

← →
_xxx_ (2007-11-23 12:52) [147]

> vpbar © (23.11.07 12:30) [144]
> >>_xxx_ (23.11.07 12:00) [139]Каких ресурсов? Если вы
> о скорости, то об этом пусть компилятор заботится. А если
> сделать так как вы сказали, то логика немного теряется,
> по-моему. Хотя этот кусок кода вообже нехороший.

Точно компилятор позаботится? Что-то не уверен я в этом.

← →
_xxx_ (2007-11-23 13:29) [148]

немного беру слова обратно:)... Компилятор оптимизирует, хотя 4 лишние команды таки добавляется.

> Да и не было проблем с пониманием кода. В свое время влился
> в проект на 4 метра исходников примерно за месяц, почти
> при полном отсутствии комментариев.С тех пор и не особо
> воспринимаю необходимость комментариев.

Через годик-два загляни в сорсы этого проекта. Восприятие изменится.

> [149] Некто © (23.11.07 13:35)

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

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