Last week, we saw some possibly ancient Pascal code. Leilani sends us some more… modern Pascal to look at today.
This block of code comes from a developer who has… some quirks. For example, they have a very command-line oriented approach to design. This means that, even when making a GUI application, they want convenient keyboard shortcuts. So, to close a dialog, you hit "CTRL+C", because who would ever use that keyboard shortcut for any other function at all? There's no reason a GUI would use "CTRL+C" for anything but closing windows.
But that's not the WTF.
procedure TReminderService.DeactivateExternalusers;
var
sTmp: String;
begin
// Main Site
if not dbcon.Connected then
dbcon.Connect;
if not trUpdate.Active then
trUpdate.StartTransaction;
qryUsersToDeactivate.Close;
sTmp := DateTimeToStr(Now);
sTmp := Copy(sTmp, 1, 10) + ' 00:00:00';
qryUsersToDeactivate.SQL.Text := 'Select ID, "NAME", ENABLED, STATUS, SITE, EXPIRATION '+
'from EXTERNAL_USERS ' +
'where ENABLED=1 and EXPIRATION<:EXPIRED';
qryUsersToDeactivate.ParamByName('EXPIRED').AsDateTime := StrToDateTime(sTmp);
qryUsersToDeactivate.Open;
while not qryUsersToDeactivate.Eof do
begin
qryUsersToDeactivate.Edit;
qryUsersToDeactivate.FieldByName('ENABLED').AsInteger := 0;
qryUsersToDeactivate.Post;
qryUsersToDeactivate.Next;
end;
if trUpdate.Active then
trUpdate.Commit;
// second Site
// same code which does the same in another database
end;
This code queries EXTERNAL_USERS
to find all the ENABLED
accounts which are past their EXPIRATION
date. It then loops across each row in the resulting cursor, updates the ENABLED
field to 0
, and then Post
s that change back to the database, which performs the appropriate UPDATE
. So much of this code could be replaced with a much simpler, and faster: UPDATE EXTERNAL_USERS SET ENABLED = 0 WHERE ENABLED = 1 AND EXPIRATION < CURRENT_DATE
.
But then we wouldn't have an excuse to do all sorts of string manipulation on dates to munge together the current date in a format which works for the database- except Leilani points out that the way this string munging actually happens means "that only works when the system uses the german date format." Looking at this code, I'm not entirely sure why that is, but I assume it's buried in those StrToDateTime
/DateTimeToStr
functions.
Given that they call qryUsersToDeactivate.Close
at the top, this implies that they don't close it when they're done, which tells us that this opens a cursor and just leaves it open for some undefined amount of time. It's possible that the intended "close at the end" was just elided by the submitter, but the fact that it might be open at the top tells us that even if they do close it, they don't close it reliably enough to know that it's closed at the start.
And finally, for someone who likes to break the "copy text" keyboard shortcut, this code repeats itself. While the details have been elided by the submitter // same code which does the same in another database
tells us all we need to know about what comes next.
This post originally appeared on The Daily WTF.