О провилах хорошего тона в программировании.

← →
Rouse_ © (2004-04-07 13:23) [80]

> [78] Alex Konshin © (07.04.04 13:19)
Ну так я ж не вникал - просто указал на излишнюю захламленность кода, которую обычно убираю при помощи with...

← →
SoftX (2004-04-07 13:28) [81]

>>Agent13 © (07.04.04 12:53) [60]
>>Присоединяюсь. Здесь на форуме мастера очень часто критикуют чей-то код, а примера как НАДО писать я так и не видел. Очень хотелось бы посмотреть.

Ну мне в принципе была полезна критика именно моего кода, и его недостатков, так как часто я ориентируюсь на скорость работы и функциональность, а не на красивость исходников. Теперь буду стараться учесть и это. Я думаю, что мастера тоже код не очень хорошо пишут, иногда, думаю, жертвуют функциональности за счет красивости...

← →
}|{yk © (2004-04-07 13:30) [82]

А я код вообще не форматирую. Для этого в Delphi есть DelForEx,
для оракловских скриптов - в SQL Navigator Source Formatter
Получается правильное и красивое оформление. А если ктото чскажет что лучше ручками форматировать, то это будет бред - зачем будучи программистом не упрощать себе жизнь?

← →
Dmitriy O. © (2004-04-07 13:32) [83]

> SoftX (07.04.04 12:06)

http://dmitriyo.narod.ru/Skrin/kod.jpg

Да такой код трудно понять :))))

← →
Alex Konshin © (2004-04-07 13:34) [84]

Ты не учитываешь
а) стоимость поддержки твоего кода (а все время, "съэкономленное" тобой будет просто ничто, по сравнению с тем, сколько ты сам или другие люди потратят на разбор твоего кода).
б) стиль позволяет избегать ошибок и упрощает отладку. Я, собственно, тебе указал именно на эти недостатки.

← →
DiamondShark © (2004-04-07 13:34) [85]

if (found=True) then with UserInfo, MainData.UserQuery do begin LOGIN := FieldByName("LOGIN").AsString; USERNAME := FieldByName("USERNAME").AsString; CANDELETEFAX := FieldByName("CANDELETEFAX").AsInteger; IMMORTAL := FieldByName("IMMORTAL").AsInteger; CANGETFAX := FieldByName("CANGETFAX").AsInteger; ADMINISTRATOR := FieldByName("ADMINISTRATOR").AsInteger; // ReadCarrentSid := True; end

ЗЫ
Var Username:String;
var CUsername:longword;
Var TSid,Sid:PSID;
var Csid:longword;
var Domain:String;
Var Cdomain:longword;
Var PUse:SID_NAME_USE;
Var BlobStream:TStream;
var i:integer;
var found:boolean;

if (found=True)

Тяжёлое наследие жабы?

← →
Внук © (2004-04-07 13:36) [86]

>>Rouse_ © (07.04.04 13:23) [80]
А я вот за использование with кроме самых простейших случаев караю расстрелом через повешение. О чем, впрочем, неоднократно говорил, переубедить ни у кого не получилось.

← →
}|{yk © (2004-04-07 13:37) [87]

Игорю Шевченко:
посмотрел вашу программу "СуперМаркет"
Скажите, а зачем вам было изобретать велосипед?
Есть же специальные языки и среды для моделирования таких процессов. GPSS например.

← →
sniknik © (2004-04-07 13:39) [88]

хотите посмотреть на действительно ужастный код? или форматирование это как сказать. посмотрите этот, и все ваши претензии покажутся детским лепетом. ;о))
http://delphi.chertenok.ru/forum/my_download.php?tema=del&action=show&id=172
а мне приходилось его еще и разбирать. ;о) (там глюк был, возможно не один но мне хватило. про глюк в ридми написано)

← →
Игорь Шевченко © (2004-04-07 13:49) [89]

}|{yk © (07.04.04 13:37)

1) Вопрос к Джеффри Рихтеру (это его пример, переведенный мной на Delphi)
2) Иллюстрация работы с объектами синхронизации в многопоточном приложении

← →
passlight © (2004-04-07 13:56) [90]

DiamondShark © (07.04.04 13:34) [85]
Похоже на VBA...

← →
Romkin © (2004-04-07 13:59) [91]

Внук © (07.04.04 13:36) [86]
ну-ну
with TCustomFrameShowAnswerClass(FindClass(FrameClassName)).Create(Self) do begin Parent := self; Align := alClient; ... end;
И не убедишь ты меня, что with - это плохо. Конструкция есть в языке, следовательно - пользуем :)
Единственная причина непользования - неудобно просматривать при отладке? Но у меня пошаговая отладка с просмотром переменных самый крайний случай. Обычно хватает пары точек останова, ну и Ctrl+Alt+W :)

sniknik © (07.04.04 13:39) [88] ААААА!!! :)))

← →
Rouse_ © (2004-04-07 14:01) [92]

> [86] Внук © (07.04.04 13:36)
Не в моих правилах кого-то разубеждать.

к примеру не буду я писать везде IP.<параметр>
вместо
with IP do begin Ttl := 1; Tos := 0; IPFlags := 0; OptSize := 0; Options := nil; end;

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

← →
SoftX (2004-04-07 14:02) [93]

>>DiamondShark © (07.04.04 13:34) [85]
>>Тяжёлое наследие жабы?

Вообще-то я писал и на Жабе и на C++ и на Ada и на Асме и на Прологе..., так что после изучения довольно немалого количества языков у меня сформировался свой стиль, возможно кривой, но мне все равно на чем писать(только FoxPro ненавижу). В принципе многим более непонятна логика моей программы, чем код. Так как многие вещи я реализую(для ускорения) так как обычный программист делать не стал бы, но в каждом данном случае это является более целесообразным, чем нормальный подход. Хоть это и затрудняет удобочитаемость кода, код ориентирован на одного человека...

← →
Rouse_ © (2004-04-07 14:06) [94]

> [88] sniknik © (07.04.04 13:39)
Тебе только за попытку разобраться в ЭТОМ (боюсь назвать коде) нужно давать орден Героя Соц Труда, и Красную Звезду на все пузо :)))))

← →
Игорь Шевченко © (2004-04-07 14:10) [95]

> код ориентирован на одного человека...

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

← →
Romkin © (2004-04-07 14:16) [96]

Кстати, а ведь неплохой вопрос...
имеем:
type TCustomFrameShowAnswer = class(TFrame) ... end; TFrameShowText = class(TCustomFrameShowAnswer) ... public constructor Create(AOwner: TComponent); override; ... end; Вызов FrameClassName := "TFrameShowText"; with TCustomFrameShowAnswerClass(FindClass(FrameClassName)).Create(Self) do begin Parent := self; Align := alClient; ... end;
Вопрос: конструктор Create какого класса будет вызван?

← →
mfender © (2004-04-07 14:19) [97]

panov © (07.04.04 12:37)
>mfender © (07.04.04 12:23) [38]
хотя бы тот же банальный 2 пробела перед begin
2 пробела не перед begin вставляются, а в строке, следующей после begin:

А вот я обычно пишу так:
function MyFuncDelta: Boolean; begin if a>b then begin Result := a - b; end else begin Result := b - a; end; end;

Я не прав?

← →
Внук © (2004-04-07 14:20) [98]

>>Romkin © (07.04.04 13:59) [91]
Попробую изложить свои соображения. Чтение такого кода подразумевает знание структуры используемых (иногда - специфических) VCL-классов. Что не является обязательным для чтения этого кода другим человеком (не автором), не говоря уже о собственноручно написанных классах. Поиск на всякий случай наличие в проекте глобальных переменных Parent и Align тоже не добавляет положительных эмоций. Не говоря уже о том, когда with применяется для нескольких объектов или вложенный with.
Оператор goto тоже есть в языке, так и что?
Когда сам начинал писать на С++, долго ругался по поводу отсутствия там with (до этого на TP7.0 писал). А потом понял всю прелесть, когда пришлось разбираться в сотнях килобайт чужого кода.
Перешел на Delphi - теперь для меня with не существует.

← →
Игорь Шевченко © (2004-04-07 14:20) [99]

> конструктор Create какого класса будет вызван?

TFrameShowText - он же виртуальный

← →
olookin © (2004-04-07 14:21) [100]

[32] Игорь Шевченко © (07.04.04 12:09)
SoftX (07.04.04 12:06)
> говорят тяжелый,
Справедливо говорят.

А про это что скажете?

procedure TEQAPbase.EqapAllsizediagram1Click(Sender: TObject);
var Form: TForm;
Chart: TChart;
dataser,trendser: TLineSeries;
i,j,n: integer;
str: array of string;
a,s: string;
lst: TStringList;
curdate: TDate;
vals: TIntegerArray;
p: PChar;
m: TDoubleArray;
pm: PDoubleArray;
psource,pdest: PChartSeries;
source,dest: TChartSeries;
begin
Form:=TForm.Create(Application);
with Form do begin Caption:="EqapAll execution file size diagram";
BorderIcons:=[biSystemMenu]; BorderStyle:=bsSingle;
Width:=500; Height:=200; Position:=poScreenCenter; end;
Chart:=TChart.Create(Form);
with Chart do begin Parent:=Form; Align:=alClient;
MarginTop:=3; MarginBottom:=1; MarginLeft:=1; MarginRight:=3;
View3D:=false; BevelInner:=bvNone; BevelOuter:=bvNone;
Title.Visible:=false; Legend.Visible:=false;
BackWall.Brush.Style:=bsSolid; BackWall.Color:=clWhite;
with LeftAxis do begin LabelsFont.Size:=6; Grid.Visible:=false;
Title.Caption:="Size, kB"; Title.Font.Size:=6; BorderWidth:=2; end;
with BottomAxis do begin LabelsFont.Size:=6; Grid.Visible:=false;
Title.Caption:="Date"; Title.Font.Size:=6;
DateTimeFormat:="dd.MM.yyyy"; end;
dataser:=TLineSeries.Create(Chart);
with dataser do begin ParentChart:=Chart;
XValues.DateTime:=true; SeriesColor:=clRed; LinePen.Width:=2;
with Pointer do begin Visible:=true; Style:=psCircle;
VertSize:=3; HorizSize:=3; Brush.Color:=clRed;
Pen.Color:=clBlack; end; end;
trendser:=TLineSeries.Create(Chart);
with trendser do begin ParentChart:=Chart; XValues.DateTime:=true;
SeriesColor:=clBlack; LinePen.Width:=2; end; end;

for i:=0 to Chart.SeriesCount-1 do Chart.Series[i].Clear;
if FileExists(dir+"\System\SizeDiagram.eqs") then begin
i:=0; curdate:=0; lst:=TStringList.Create;
lst.LoadFromFile(dir+"\System\SizeDiagram.eqs");
for j:=0 to lst.Count-1 do begin a:=lst.Strings[j];
s:=Copy(a,1,Pos("=",a)-1); n:=StrToInt(Copy(a,Pos("=",a)+1,Length(a)));
if StrToDate(s)<>curdate then begin SetLength(vals,i+1); vals[i]:=n;
Chart.Series[0].AddXY(StrToDate(s),trunc(vals[i]/1000));
SetLength(str,i+1); str[i]:=a; curdate:=StrToDate(s); Inc(i); end else
str[i-1]:=a; end; lst.Clear;
for i:=0 to Length(str)-1 do lst.Add(str[i]);
lst.SaveToFile(dir+"\System\SizeDiagram.eqs");
pm:=@m; source:=TChartSeries.Create(nil); source.Assign(dataser);
dest:=TChartSeries.Create(nil); dest.Assign(trendser);
psource:=@source; pdest:=@dest;
PolynomialRegression(nil,nil,psource,pdest,3,pm,clBlack,p);
trendser.Assign(dest); m:=nil; pm:=nil;
psource:=nil; source.Free; pdest:=nil; dest.Free;
ChartMinMax(Chart); end; vals:=nil; str:=nil; Form.ShowModal;
dataser.Clear; dataser.Free; trendser.Clear; trendser.Free;
Chart.FreeAllSeries; Chart.Destroy; Form.Destroy; lst.Clear; lst.Free;
end;

PS: я жуть как не люблю длинные листинги - мне нравится когда все перед глазами. Распишите представленное выше по правилам оформления кода, и у вас получится страниц 10 в доковском документе, размер шрифта 12.

← →
Игорь Шевченко © (2004-04-07 14:23) [101]

olookin © (07.04.04 14:21)

Его нельзя прочитать.

← →
Style © (2004-04-07 14:26) [102]

[100]
Мдя...
А теперь пощитайте время на его прочтение до полного осознания что там происходит :)

А отладчик вы вообще не используете???
Или к такому виду исходник приводят после отладки :)

← →
Romkin © (2004-04-07 14:27) [103]

Внук © (07.04.04 14:20) [98] Ты, эда, не усугубливай :))) Все дело в культуре: уверяю, в моем коде ты можешь быть уверен, что засады нет, переменные в блоке with либо принадлежат этому объекту, либо локальные данного метода (очень редко на F - приватные). Все.
Иначе ты увидишь полную расшифровку... Полностью там написано:
with TCustomFrameShowAnswerClass(FindClass(FrameClassName)).Create(Self) do begin Parent := self; Align := alClient; ShowAnswers(DMTasks.cdsQuestion.FieldByName("QUEST_KIND").asString); end;
Неужели код плох для чтения?

← →
Anatoly Podgoretsky © (2004-04-07 14:28) [104]

mfender © (07.04.04 14:19) [97]
Право имеешь, но логически как бы добавляешь новые уровни вложенности, там где их нет.

← →
Style © (2004-04-07 14:31) [105]

>>[100]
Form:=TForm.Create(Application);
....
Form.Free;

таже беда где у тя try finally????

← →
VAleksey © (2004-04-07 14:34) [106]

> olookin © (07.04.04 14:21) [100]

Вот матерится будет тот кому это придется разгребать.

← →
Внук © (2004-04-07 14:34) [107]

>>Romkin © (07.04.04 14:27) [103]
Дык, я и говорю - если аккуратно :))) Иначе - расстрел, с занесением в личное дело. А что до меня касается - вообще не применяю такого.

← →
mfender © (2004-04-07 14:35) [108]

> Anatoly Podgoretsky © (07.04.04 14:28) [104]
> mfender © (07.04.04 14:19) [97]
> Право имеешь, но логически как бы добавляешь новые уровни
> вложенности, там где их нет.

Нет, ну приведенный пример, конечно, содержит лишние begin...end, для одного движения я, разумеется, не стану делать блок. Но в целом я так всегда делаю. Просто самому понятней (я - не программист, это мое хобби. Пишу только для себя. Вот последнее творение - админский клиент для сайта, БД обслуживать.) Да, еще когда пишу, обычно сразу пишу begin...end, чтобы потом не было мучительно трудно концы искать.

Всегда пишу скобки парой, а begin...end это операторные скобки
Также поступаю и с другими конструкциями, типа try, case и подобное, уменьшает количество ошибок.

Кстати сабж. интересный
О провилах! Или вилы хорошего тона :)

Это грабли только плохие бывают, а вилы нет.

> Это грабли только плохие бывают, а вилы нет.

А я и написал - вилы хорошего тона!

Вот я щас на грабли чуть не наступил :)

Всего лишь то забыл в DotReplace result := ""; в
начале функции поставить :)
Дык у меня сейчас такие миллиарды в банк пошли :)
//Экспорт в БОГОРДСК БАНК! procedure TFrm_Export.Bogorodsk(); var Str: TStringList; f,i: integer; function DotReplace(Str: string):string; var i: integer; begin for i := 1 to Length(Str) do begin if(Str[i] = ",") then result := result + "." else result := result + Str[i]; end; end; begin if(not SaveDialog.Execute) then Exit; if(FileExists(SaveDialog.FileName)) then begin if(MessageBox(Handle,"Файл существует. Заменить?","Сообщение", MB_ICONQUESTION or MB_YESNO) = ID_NO) then exit else deletefile(SaveDialog.FileName); end; Str := TStringList.Create(); try qry_export.Close; qry_export.ParamByName("DateOper").AsDate := CurrentDate; qry_export.ParamByName("Flag").AsInteger := 0; qry_export.ParamByName("BankID").AsFLoat := qry_banklist.FieldByName("BankID").AsFLoat; OpenQueryWait(nil,qry_export,STATE_OPEN); qry_export.first(); for i := 1 to qry_export.RecordCount do begin Str.Add("#"+IntTOStr(i)+"|PayDocRuXE"); for f := 0 to Qry_Export.Fields.Count-1 do begin if(qry_export.fields[f].DataType = ftFloat) then begin Str.Add( WinToDos(DotReplace(qry_export.fields[f].AsString))); end else Str.Add( WinToDos(qry_export.fields[f].AsString)); end; Str.Add(";end"); qry_export.Next(); end; Str.SaveToFile(SaveDialog.FileName); ShowMessage("Экспорт успешно завершен!" + CHR($0D) + CHR($0A) + "Экспортировано записей: " + IntToStr(qry_export.RecordCount)); finally str.Free; end; end;

> olookin © (07.04.04 14:21)
Да такой код не смогла обработать до конца даже моя AvtoShema.
http://dmitriyo.narod.ru/Skrin/olookin.jpg

> function DotReplace(Str: string):string;
> var
> i: integer;
> begin
> for i := 1 to Length(Str) do
> begin
> if(Str[i] = ",") then
> result := result + "."
> else
> result := result + Str[i];
> end;
>
> end;
>

Result := StringReplace(Str, ",", ".", [rfReplaceAll]);

> olookin © (07.04.04 14:21) [100]

Ха-ха 3 раза :)
Это все детский сад. Учу, пока я жив:

#include #include #define m(x)(x#define g tj()-J #define a(x)(x#define h(x)((x)#define f 9999 #define A return #define H printf( #define R double #define U int #define V for #define b else #define u while #define B if U v,w,Y= -1,W,J,p,F,o=f,M,N,K,X,YY,_,P[f],s(); typedef U(*L)(); L q[f]; tj(){ U S=m(v)+(m(w) v?a(v)>1||w-Y||!q[J]:(w-Y (w-Y*2||q[W+Y*(N+1)]|| (J>>K)-K+(Y-1)/ v?a(v)>2))||q[J]; } z(){ _=5; A v*w||g; } e(){ _= -2; A(v*v*v-v||w*w*w-w) (J-W-2||(W N)-4||(W>>K!=(Y-1?N:0))|| q[W+1]||q[W+2]||q[W+K]!=z||P[W+K]*YA(R)(rand() PZ)/(R)PZ; } l(){ _=K+1; A(v*w a(v)-a(w))||g; } R UC(){ R i=0,d; u((i+=d=VR())J=n; B(P[W]!=Y||P[J]==Y)A J+1; v=(J N)-(W N); w=(J>>K)-(W>>K); A q[W]()||(x QL(W,J,s)); } TT(W){ v=w=0; A q[W]()+K; } s(){ U j= -1,i; Y= -Y; V(i=0; i B(j>=0 !I(i,j,0))A Y= -Y; } A!(Y= -Y); } bb(){ _=1; A a(v*w)-2; } uv(){ V(v=0; v>K)==0){ U S=h(v N); q[v]=!S?z:(S==1?bb:(S==2?c:(v N>K?l:e))); } b B(h(v>>K)==1)q[v]=k; b q[v]=0; P[v]=!!q[v]*(28-v); } } y(){ U G=Y,i; J=0; V(i=0; ii%8||H"\n%4o ",i); B((Y=P[i]=m(P[i])) TT(i))H"%c ",_+93+Y*16); b H"- "); } H"\n "); do H"%2d",i++ N); u(i N); Y=G; H"\n"); } O(W,J){ B((q[J]=q[W])==k h(J>>K)==0)q[J]=l; B(q[W]==e)B(J-W==2)O(J+1,J-1); b B(W-J==2)O(W-1,W+1); P[J]=P[W]; q[W]=0; P[W]=0; } QL(W,J,D)L D; { U HQ=P[J],YX; L AJ=q[J],XY=q[W]; O(W,J); YX=D(); O(J,W); q[J]=AJ; q[W]=XY; P[J]=HQ; A YX; } C(){ U i,j,BZ=0; V(i=0; i r=h(i>>K)+h(i N),G=Y, S=Z==z?88:(Z==k?11 +r+(P[i]>K):(i>>K)): (Z==l?124-((YY>K)!=(P[i]>0?0:N)))?M:0): (Z==c?41+r:(Z==e?f-r-r:36+r+r)))); Y=P[i]; V(j=0; j++j)B(!I(i,j,0))S+=(P[j]?5:1); BZ+=G==Y?S:-S; Y=G; } } B(!(++X M-1))write(1,".",1); A BZ; } PX(){ U i,Q=0,XP=0,JZ=M*M,E= -f,t,S=o; B(!F--)A++F+C(); V(i=0; i>K+K,i M-1,1)){ Y= -Y; o= -E; t= -QL(i>>K+K,i M-1,PX); Y= -Y; B(t>E){ ++XP; Q=i; E=t; B(E>=S) A++F,E; } } B(!XP)E=s()?-f+1:0; p=Q; A++F,E; } RZ(){ U i,j,T=0; V(; ; ){ y(); o=f; do{ H"\n%d %d %d %s ",X,T,C(),s()?"!":">"); fflush(stdout); } u(scanf("%o%o", i, j)!=2||I(i,j,1)); O(i,j); y(); X=0; ++YY; Y= -Y; T=PX(); i=p>>(KB(T>M*M)H"\nHar har.\n"); } } main(ac,av)char**av; { long B(T>time(),j=time( j); R i=0; srand((U)j); V(M=0; MV(N=1; N*N1?atoi(av[1]):2; uv(); RZ(); }

Кстати, полностью рабочий код. =))

> if(not SaveDialog.Execute) then Exit;
>
> if(FileExists(SaveDialog.FileName)) then
> begin
> if(MessageBox(Handle,"Файл существует. Заменить?","Сообщение",
> MB_ICONQUESTION or MB_YESNO) = ID_NO) then
> exit
> else
> deletefile(SaveDialog.FileName);
> end;

SaveDialog.Options := SaveDialog.Options + [ofOverwritePrompt]

Nikolay M. © (07.04.04 15:11)

Suxx. Оцени:

/* * Program to compute an approximation of pi * by Brian Westley, 1988 * (requires pcc macro concatenation; try gcc -traditional-cpp) */ #define _ -F<00||--F-OO--; int F=00,OO=00; main(){F_OO();printf("%1.3f\n",4.*-F/OO/OO);}F_OO() { _-_-_-_ _-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_-_-_-_-_ _-_-_-_-_-_-_-_ _-_-_-_ }

О провилах хорошего тона в программировании. Найти похожие ветки