Today, fine people, I present to you what I believe to be the biggest example of WTF code in the history of WTF code.
Today, fine people, I present to you what I believe to be the biggest example of WTF code in the history of WTF code.
Correct me if I am wrong, but this code merely fills an array with 0 to 255:
procedure PPLoadSQLCollation(cn : ADOWEConnection = nil);
var ss : AnsiString;
rs : ADSRecordset;
iRA : OleVariant;
ii, iPos : integer;
begin
if pArySQLCollation <> nil then Dispose(pArySQLCollation);
pArySQLCollation := nil;
if cn = nil then
cn := GlobalADOConnection;
Assert(cn <> nil);
ss := 'declare @tbl table (iChr int) ' + crlf +
'declare @ii varchar(4000) ' + crlf +
'set @ii= 0 ' + crlf +
'while @ii<= 255 ' + crlf +
'begin ' + crlf +
' insert into @tbl values(@ii)'+ crlf +
' set @ii=@ii+ 1 ' + crlf +
'end ' + crlf +
'select * into dbo.TBL_ADS_CONFIG_SQL_COLLATION from @tbl';
TQueryRunner.ExecuteQueryNoMsg(ss,iRA, rs,cn);
if TQueryRunner.ExecuteQuery('SELECT * FROM TBL_ADS_CONFIG_SQL_COLLATION ORDER BY CHAR(ICHR)',
iRA, rs, cn) then
begin
New(pArySQLCollation);
ii := 0;
while not rs.EOF do
begin
iPos := rs.Fields['iChr'].Value;
pArySQLCollation[iPos] := ii;
inc(ii);
rs.MoveNext;
end;
rs.Close;
end;
end; // PPLoadSQLCollation
Correct me if I am wrong, but this code merely fills an array with 0 to 255:
procedure PPLoadSQLCollation(cn : ADOWEConnection = nil);
var ss : AnsiString;
rs : ADSRecordset;
iRA : OleVariant;
ii, iPos : integer;
begin
if pArySQLCollation <> nil then Dispose(pArySQLCollation);
pArySQLCollation := nil;
if cn = nil then
cn := GlobalADOConnection;
Assert(cn <> nil);
ss := 'declare @tbl table (iChr int) ' + crlf +
'declare @ii varchar(4000) ' + crlf +
'set @ii= 0 ' + crlf +
'while @ii<= 255 ' + crlf +
'begin ' + crlf +
' insert into @tbl values(@ii)'+ crlf +
' set @ii=@ii+ 1 ' + crlf +
'end ' + crlf +
'select * into dbo.TBL_ADS_CONFIG_SQL_COLLATION from @tbl';
TQueryRunner.ExecuteQueryNoMsg(ss,iRA, rs,cn);
if TQueryRunner.ExecuteQuery('SELECT * FROM TBL_ADS_CONFIG_SQL_COLLATION ORDER BY CHAR(ICHR)',
iRA, rs, cn) then
begin
New(pArySQLCollation);
ii := 0;
while not rs.EOF do
begin
iPos := rs.Fields['iChr'].Value;
pArySQLCollation[iPos] := ii;
inc(ii);
rs.MoveNext;
end;
rs.Close;
end;
end; // PPLoadSQLCollation
Looking at the name, doesn't it actually try to retrieve the collation order from the DB? If so for case-sensitive English collation it would be 0..255 but for other collations it would be the same values but ordered differently.
ReplyDeleteyup, I don't get why the query must be so complex, why wouldn't
ReplyDeleteINSERT INTO X(Y, Z, Q, ...) SELECT Y, Z, Q FROM FF;
SELECT * FROM X;
work?
Why wouldn't
ReplyDeletefor ii := 0 to 255
work? ;-)
Actually I see now that they key to this thing is
ReplyDeleteORDER BY CHAR(ICHR)
But still......
Nick Hodges if the code is determining collation order, "for ii := 0 to 255" would only "work" for english case sensitive collation (but wouldn't for English case insensitive, or Russian, or Spanish, or accent insensitive Unicode, etc.).
ReplyDeleteThe good question is whether your app is international, and whether the above code was tested in international conditions with different collations :)
Also it was probably written in a pre-Unicode era, given the 255 limit. But the 255 limit would work ok for "most" of Latin-based languages.
ReplyDeleteOur app is most definitely not internationalized, but the query does return a non-ordered result.
ReplyDeleteIt still seems a strange way to do things....
I take it all back now -- I see what it is doing.......
ReplyDeleteIT's ordering the array by character values, so that, say, all the U, u and u with an umlaut are all in order.
Never mind. :-)
ReplyDeleteThen your DB is probably set to English case-insensitive collation (and/or maybe spanish, if you're in the US?)
ReplyDeleteThere are probably other ways by directly checking the value of the TBL_ADS_CONFIG_SQL_COLLATION and then using pre-generated collation tables, but they won't necessarily be simpler ways, and they would be problematic if the pre-generated collation tables don't match the SQL ones (says the SQL collation tables are updated/fixed in a patch, but your local ones are not).
Letting the DB do the ordering and then using it is probably the safer way, I know an app that did not do that and had problems when the Euro symbol got added...
Some comments would have been nice though, so you could more clearly see why this "overcomplication" was done. So in that regard, still a WTF.
ReplyDeleteAsbjørn Heid The name of the function with "LoadSQLCollation" was well chosen and unambiguous however.
ReplyDeleteThough "Collation" is quite on the technical jargon side, so it's probably deserving of a "scarecrow" header ;-)
Uncommented complications: It does seem to fail the "code as if the subsequent maintainer is a psychopath with your address and a large axe" test.
ReplyDeleteIt's my favourite rather dark description of how to tell good code.
Eric Grange A couple of words of why this is needed wouldn't be out of place as well. Perhaps the assumptions or requirements have changed since it was written.
ReplyDeleteAsbjørn Heid Possibly, but the most complicated stuff in that function are the ADO bits and the lack of multi-line string support in the Delphi compiler which makes the SQL look more complicated than it is.
ReplyDeleteThe rest probably belongs to a higher level description of why the collation is needed on the application side, assuming it's more exotic than interactive client-side sorting.
At some point, you can't and shouldn't define every technical jargon in the comments, and I would expect TBL_ADS_CONFIG_SQL_COLLATION to be documented at the DB level (and not in the code).
That said, how would you comment that function without paraphrasing the function name, explaining what a collation is or copy-pasting the DB docs?