критика кода DMClient

← →
nikkie © (2004-08-12 11:45) [0]

тема, родившаяся в недрах (со 114-го по 128-й примерно пост)
http://delphimaster.net/view/14-1091941670/

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

← →
Sandman25 © (2004-08-12 11:48) [1]

AssignFile и Rewrite рулят.
CreateFile может стать deprecated в новых версиях виндоус :)

← →
Думкин © (2004-08-12 11:49) [2]

Можно чтобы "Написать ответ" было внизу окна всегда или по желанию?
А то иногда прыгать приходится по ветке и набирать корзину для ответа, а потом уже отвечать скопом.
Или никак?

← →
nikkie © (2004-08-12 11:50) [3]

по поводу AssignFile и т.д. мы вроде разобрались - это личная неприязнь к этим функциям у Fay. теперь хотелось бы разъяснить такой момент.

>Fay
>А вычисление количества строк через ReadLn в цикле - просто шедевр.
какие есть альтернативы?

← →
Sandman25 © (2004-08-12 11:50) [4]

[2] Думкин © (12.08.04 11:49)

Запускай 2 клиента :)

← →
Anatoly Podgoretsky © (2004-08-12 11:52) [5]

nikkie © (12.08.04 11:45)
Обычно этот процесс называют - порка <наименование>, эта терминология уже прижилась.

← →
Danilka © (2004-08-12 11:53) [6]

[3] nikkie © (12.08.04 11:50)
> какие есть альтернативы?

Например, читать файл посимвольно и подсчитывать #13#10.

[4] Sandman25 © (12.08.04 11:50)
Попробуй, не запускается. :))

← →
Sandman25 © (2004-08-12 11:55) [7]

[6] Danilka © (12.08.04 11:53)

Упс. Предыдущая версия запускалась. Зря я экзешник не переименовал при апгрейде, пригодился бы.

← →
Anatoly Podgoretsky © (2004-08-12 11:59) [8]

nikkie © (12.08.04 11:50) [3]
Нет альтернативы

Danilka © (12.08.04 11:53) [6]
Работать с текстовыми файлами как с двоичными конечно можно, но не кошерно.

← →
nikkie © (2004-08-12 12:02) [9]

>Думкин
>Можно чтобы "Написать ответ" было внизу окна всегда или по желанию?
идея понятна, можно подумать на эту тему.

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

о программе
http://schachspieler.narod.ru/dmclient.html
скачать исходники можно здесь:
http://prdownloads.sourceforge.net/dmclient/dmclient_2.1.2_src.zip?download

← →
nikkie © (2004-08-12 12:07) [10]

>Anatoly Podgoretsky
хорошо, назовем это:
"публичная порка меня родимого"
с хитрым прицелом...

>Sandman25
>Запускай 2 клиента :)
есть проще и удобнее вариант - notepad или какой там твой любимый текстовый редактор в качестве второго окна.

← →
Piter © (2004-08-12 12:27) [11]

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

P.S. Не забывайте, Nikkie - мастер, хоть и не засвеченный, поэтому туфты в коде не напишет :)

← →
Piter © (2004-08-12 12:27) [12]

nikkie © (12.08.04 12:07) [10]
есть проще и удобнее вариант

а еще проще и удобнее - сделать как я сделал в клиенте

← →
Fay © (2004-08-12 12:42) [13]

2 nikkie © (12.08.04 11:50) [3]

function GetLineCount(const cFileName : string) : Integer; var h, m, sz : DWORD; v, p, pe : PChar; begin Result := 0; h := CreateFile(PChar(cFileName), GENERIC_READ, FILE_SHARE_READ, nil, OPEN_EXISTING, 0, 0); if h = INVALID_HANDLE_VALUE then Exit; m := 0; v := nil; try sz := GetFileSize(h, nil); if (sz = INVALID_FILE_SIZE) or (sz = 0) then Exit; m := CreateFileMapping(h, nil, PAGE_READONLY, 0, 0, nil); if m = 0 then Exit; v := MapViewOfFile(m, FILE_MAP_READ, 0, 0, 0); if v = nil then Exit; p := v - 1; pe := p + sz; if pe^ <> #10 then Result := 1; repeat Inc(p); if p^ = #13 then begin Inc(Result); if p[1] = #10 then Inc(p); end; until p >= pe; finally if v <> nil then UnmapViewOfFile(v); if m <> 0 then CloseHandle(m); CloseHandle(h); end; end;

← →
Fay © (2004-08-12 12:50) [14]

2 Anatoly Podgoretsky © (12.08.04 11:59) [8]
Не кошерно работать с многозадачной, имеющей продвинутую систему прав, ОС так же, как под "ДОСкой". А вещи типа "если файл есть, то открыть, иначе - создать" пишутся одной функцией CreateFile. Но говорить об это не принято. Идиотизм.

← →
Fay © (2004-08-12 12:59) [15]

2 Piter © (12.08.04 12:27) [11]
Глянь на GetLastQuestionTimeStamp.

З.Ы.
В целом - программа ничего. Просто не нравится. Особенно работа с текстом и файлами.

← →
Anatoly Podgoretsky © (2004-08-12 13:03) [16]

Fay © (12.08.04 12:50) [14]
Можно, но придется дублировать работу, которая уже проделана разработчиками Паскаля. Если есть желание то почему бы и нет.

← →
nikkie © (2004-08-12 13:05) [17]

я так и думал, что предложишь через FileMapping сделать. наверное, быстрее получится, интересно проверить насколько. думаю, что пользователь не заметит разницы. есть другое место, где гораздо более серьезен вопрос производительности - SaveNewQuestions. к сожалению, так, как сейчас хранится ускорить существенно не получится (хотя к моему удивлению все равно работает довольно шустро, хотя и видна разница между тем, когда есть обновления и когда их нет - подтормаживание в пределах секунды). надо было бы сделать хранение тем в обратном порядке, можно было бы сделать быстрее.

>Идиотизм.
ну зачем так... кому-то может вопрос кросс-платформенности важнее.

← →
Fay © (2004-08-12 13:08) [18]

2 Anatoly Podgoretsky © (12.08.04 13:03) [16]
Это не дублирование. Почти ничего общего. А вот сделать раз в 20 быстрее - почему бы и нет? С тому же, использовать CreateFile под Windows - нормально и правильно.

← →
Fay © (2004-08-12 13:09) [19]

2 nikkie © (12.08.04 13:05) [17]
Не скинешь ссылочку на кроссплатформенный MSHTMP_tbl.pas ? Очень надо 8)

← →
Danilka © (2004-08-12 13:19) [20]

[18] Fay © (12.08.04 13:08)
> А вот сделать раз в 20 быстрее - почему бы и нет?

Чем тестировал? Откуда такие результаты, что через CreateFile в 20 раз быстрее чем через по-обычному, для паскаля, способу?

← →
nikkie © (2004-08-12 13:24) [21]

>раз в 20 быстрее
ой ли? сейчас будем проверять...

>GetLastQuestionTimeStamp
что там не понравилось?

>Не скинешь ссылочку на кроссплатформенный MSHTMP_tbl.pas ? Очень надо 8)
я иронию уловил... тебя не удивляет, что работа с сервером вынесена DMCServer.pas, работа с файлами собрана в DMCStorageFile.pas, за каким-то лешим еще сделан DMCStorage.pas, а не навалено все в Main.pas? в той ветке мы обсуждаем возможность создания клиента в виде web-сервера или news-gate. ты думаешь, что этот код не мог бы быть повторно использован для такого приложения, будучи откомпилированным FPC или Kylix?

так что MSHTML_TLB.pas не абсолютно необходим... и потом, ты ведь не только меня ругаешь, а всех, использующих AssignFile и co.

← →
Fay © (2004-08-12 13:26) [22]

2 Danilka © (12.08.04 13:19) [20]
Присылай пример. Сравним. А то так можно кому хочешь сказать "У мене типа с пол-метра, не меньше".

2 nikkie © (12.08.04 13:05) [17]
MSHTMP_tbl.pas следует читать как MSHTML_tbl.pas

← →
Danilka © (2004-08-12 13:31) [23]

[22] Fay © (12.08.04 13:26)
У меня нет примера. Но видимо у тебя есть, раз ты так смело говоришь, что в 20 раз быстрее. Поэтому я и хотел узнать, откуда ты взял эту цифру, как тестировал.

← →
Danilka © (2004-08-12 13:32) [24]

[21] nikkie © (12.08.04 13:24)
В качестве оффтопика, у тебя под какую версию дельфи клиент писан?

← →
Fay © (2004-08-12 13:35) [25]

Это цифра из сравнения способов вычисления количества строк в файле. А TDMCStorageFile.GetLastQuestionTimeStamp думаю раз в 100 можно ускорить. Если не больше.

← →
Fay © (2004-08-12 13:37) [26]

2 Sandman25 © (12.08.04 11:48) [1]
Надо иметь неслабое здоровье, чтобы дожить до этого.

← →
nikkie © (2004-08-12 13:56) [27]

>GetLastQuestionTimeStamp думаю раз в 100 можно ускорить
каким образом? пиши свою версию, сравним.

← →
Fay © (2004-08-12 13:57) [28]

ОК

>Danilka
>под какую версию дельфи клиент писан?
исходники компилируются под D5-D7 при условии, что установлены Indy и EmbeddedWB.

Danilka © (12.08.04 13:19) [20]
Проверка скорости загрузки списка строк операциями ReadLn и потоковыми, показывала неодназначные результаты, раз на раз не приходился. И никакими 20 разами и близко не пахнет. Красота работы со строками у ReadLn большая, а когда оказывается нужным сделать например такое ReadLn(F, IntVar, FloatVar, BooleanVar, DateVar, SteVar) то особо проявляется, сложность создания такого же но не средствами ReadLn ни как не может окупить даже возможное некоторое замедление.
Ну и самое простое если кому то не нравится поддержка текстовых и типизированых файлов в Паскале, а есть желание разработать свое, то это его право, было бы желание, а запрета нет.

На Фрипаскале написал:
Program test2; Uses SysUtils; function GetCPUTick: int64; assembler; asm db 0fh,31h; end; Function GetNumStrings(Var P : Pointer; Size : Longint) : Longint; Var Num : Longint; i : Longint; begin Num:=0; for i:=0 to Size-1 do begin if Byte(Pointer( Longint(P)+i )^)=10 then inc(Num); end; GetNumStrings:=Num; end; Var Time : Int64; F : File; P : Pointer; Size : Longint; NumStrings : Longint; BEGIN Time:=0; if ParamCount>0 then if FileExists(ParamStr(1)) then begin Time:=GetCPUTick; AssignFile(F,ParamStr(1)); Reset(F,1); Size:=FileSize(F); GetMem(P,Size); BlockRead(F,P^,Size); NumStrings:=GetNumStrings(P, Size); FreeMem(P,Size); CloseFile(F); Time:=GetCPUTick-Time; end; WriteLn("File: ",ParamStr(1)); WriteLn("Number of strings - ",NumStrings); WriteLn("Speed CPU ticks - ",Time); WriteLn; WriteLn("Press ENTER:"); ReadLn; END.

Результат по файлу этого же исхождника:

File: test2.pas Number of strings - 60 Speed CPU ticks - 544590 Press ENTER:

Напиши чтобы в 20 раз быстрее было?

cyborg © (12.08.04 14:08) [32]
Писать по требованию надо, что бы в 20 раз медленнее было!!! Такого было утверждение. Где у тебя проверка с ReadLn? Я к тому же не заостряю внимание на том, что код неправильный, разделитель строк не #10, а #13#10.

> [34] Anatoly Podgoretsky © (12.08.04 14:14)

проверка разделителей как раз правильна, это в ДОСе было только #13#10, сейчас достаточно искать только #10, оно и в таком и в таком формате разделителя присутствует.

> [33] Sandman25 © (12.08.04 14:10)

10 гигов не каждая файловая система держит. Я знал, что акой пример приведут, достаточно переделать на чтение по частям, функция таже, только несколько раз вызывать с новыми читаемыми данными и складывая реультат, к примеру по 4 килобайта достаточно для экономии памяти?

тестовая программа:
program LineCount; {$APPTYPE CONSOLE} uses SysUtils, Windows; function GetLineCountStd(const FileName : string) : Integer; var F: TextFile; s: String; begin Result := 0; if not FileExists(FileName) then begin Exit; end; AssignFile(F, FileName); Reset(F); try while not EOF(F) do begin Readln(F, s); Inc(Result); end; finally CloseFile(F); end; end; function GetLineCountFay(const cFileName : string) : Integer; var h, m, sz : DWORD; v, p, pe : PChar; begin Result := 0; h := CreateFile(PChar(cFileName), GENERIC_READ, FILE_SHARE_READ, nil, OPEN_EXISTING, 0, 0); if h = INVALID_HANDLE_VALUE then Exit; m := 0; v := nil; try sz := GetFileSize(h, nil); if (sz = INVALID_FILE_SIZE) or (sz = 0) then Exit; m := CreateFileMapping(h, nil, PAGE_READONLY, 0, 0, nil); if m = 0 then Exit; v := MapViewOfFile(m, FILE_MAP_READ, 0, 0, 0); if v = nil then Exit; p := v - 1; pe := p + sz; if pe^ <> #10 then Result := 1; repeat Inc(p); if p^ = #13 then begin Inc(Result); if p[1] = #10 then Inc(p); end; until p >= pe; finally if v <> nil then UnmapViewOfFile(v); if m <> 0 then CloseHandle(m); CloseHandle(h); end; end; type TGetLineCountFunc = function (const cFileName : string) : Integer; const TEST_NUM = 100; TEST_FILE = "C:\temp\list.txt"; procedure Test(f: TGetLineCountFunc; const FileName: string; Desc: string); var dwStart, dwEnd: DWORD; i: integer; begin dwStart := GetTickCount; for i := 0 to TEST_NUM do begin f(FileName); end; dwEnd := GetTickCount; Writeln(Desc, dwEnd - dwStart, " ticks spent"); end; begin Test(GetLineCountStd, TEST_FILE, "Std:"); Test(GetLineCountFay, TEST_FILE, "Fay:"); end.

тестовые файлы - это списки тем из моего архива за год и 3 месяца, которые существует клиент. те самые файлы, для которых эта функция и вызывается реально.

"Потрепаться" (размер 5181701 байт, 16256 строк).
Std:11687 ticks spent
Fay:1266 ticks spent

"Основная" (размер 6342060 байт, 20328 строк).
Std:14328 ticks spent
Fay:1532 ticks spent

резюме: работает почти в 10 раз быстрее, 0.01 секунды вместо 0.1 секунды.

btw, если в файле последний символ 0x12, в твоей функции все пучком будет?

nikkie © (12.08.04 14:23) [37]
Упрости Readln(F);
и поменяй местами

Test(GetLineCountStd, TEST_FILE, "Std:");
Test(GetLineCountFay, TEST_FILE, "Fay:");

тьфу, не 0x12, а 0x13, конечно.
по-моему строку
if p[1] = #10 then Inc(p);
выкинуть надо, а 0x13 заменить на 0x10.

критика кода DMClient Найти похожие ветки