I asked ChatGPT what’s wrong with the following Delphi code

Slashdot just posted "What Happens When ChatGPT Can Find Bugs in Computer Code?" which links a PC Mag article with the title "Watch Out, Software Engineers: ChatGPT Is Now Finding, Fixing Bugs in Code"

So I asked ChatGPT “What’s wrong with the following Delphi code:”

function IsoStringToDateTime(const ISODateTime: string): TDateTime;
const
  ISOShortLen = 19;
  ISOFullLen = 23;
var
  y, m, d, h, n, s, z: Word;
begin
  // ISODateTime should be in one of these formats:
  // YYYY-MM-DDTHH:NN:SS, YYYY-MM-DD HH:NN:SS
  // YYYY-MM-DDTHH:NN:SS.ZZZ, YYYY-MM-DD HH:NN:SS.ZZZ
  if (Length(ISODateTime) <> ISOShortLen) and (Length(ISODateTime) <> ISOFullLen) then
    raise EConvertError.Create('Invalid ISO date time string: ' + ISODateTime);
  y := SysUtils.StrToInt(Copy(ISODateTime,  1, 4));
  m := SysUtils.StrToInt(Copy(ISODateTime,  6, 2));
  d := SysUtils.StrToInt(Copy(ISODateTime,  9, 2));
  h := SysUtils.StrToInt(Copy(ISODateTime, 12, 2));
  n := SysUtils.StrToInt(Copy(ISODateTime, 15, 2));
  s := SysUtils.StrToInt(Copy(ISODateTime, 18, 2));
  z := StrToIntDef(Copy(ISODateTime, 21, 3),  0); // Optional
  Result := EncodeDate(y, m, d) + EncodeTime(h, n, s, z);
end;

(That code is from GExperts’ GX_GenericUtils unit.)

It answered this:

The code does not account for timezone information in the input string and assumes the time is in UTC.

Which is correct, although not really relevant in the context of GExperts.

And after I clicked “Regenerate Response”

The code may raise an exception if the input string length is neither 19 nor 23, which doesn’t match the expected ISO 8601 format. The code should also validate the input string to ensure it contains only valid characters, such as numbers and the ‘T’ or ‘ ‘ separator, and that the date and time components are within a valid range.

Correct, but the comment of course states exactly which formats are supported. It should possibly check for ‘T’ or ‘ ‘ though.

And again (which just rephrases the last sentence).

The code does not handle invalid date and time values that are provided in the input ISO string (e.g. day of the month larger than 31, minute larger than 59).

Not wrong, but the EncodeDate and EncodeTime functions should take care of that.

So, was it helpful? Maybe. It at least got me thinking about the possible shortcomings of the code. But on the other hand, it might waste developer time with “bugs” like this that aren’t really bugs because of the context the code is used in.

I also asked it “How would you fix that?”

The code can be fixed by adding checks to validate the values of year, month, day, hour, minute, second and milliseconds before encoding the date and time.

For example, the following code can be used to validate the values of day and month:

if (d < 1) or (d > DaysInAMonth(y, m)) then
  raise EConvertError.Create('Invalid day of the month: ' + IntToStr(d));
if (m < 1) or (m > 12) then
  raise EConvertError.Create('Invalid month of the year: ' + IntToStr(m));

And the following code can be used to validate the values of hour, minute and second:

if (h > 23) then
  raise EConvertError.Create('Invalid hour of the day: ' + IntToStr(h));
if (n > 59) then
  raise EConvertError.Create('Invalid minute of the hour: ' + IntToStr(n));
if (s > 59) then
  raise EConvertError.Create('Invalid second of the minute: ' + IntToStr(s));

(As baka0815 pointed out in Delphi Praxis, there is a bug: The check for the day uses DaysInAMonth and passes y and m as parameters. But these could be wrong because they are checked only later.)

Not too bad either, it didn’t use CreateFmt though. Which gave me another idea: “Change that code to use GnuGettext for translating the error messages.”

The following code can be used to translate the error messages using GnuGettext:

function IsoStringToDateTime(const ISODateTime: string): TDateTime;
const
  ISOShortLen = 19;
  ISOFullLen = 23;
var
  y, m, d, h, n, s, z: Word;
begin
  if (Length(ISODateTime) <> ISOShortLen) and (Length(ISODateTime) <> ISOFullLen) then
    raise EConvertError.Create(_('Invalid ISO date time string: ') + ISODateTime);
  y := SysUtils.StrToInt(Copy(ISODateTime,  1, 4));
  m := SysUtils.StrToInt(Copy(ISODateTime,  6, 2));
  d := SysUtils.StrToInt(Copy(ISODateTime,  9, 2));
  h := SysUtils.StrToInt(Copy(ISODateTime, 12, 2));
  n := SysUtils.StrToInt(Copy(ISODateTime, 15, 2));
  s := SysUtils.StrToInt(Copy(ISODateTime, 18, 2));
  z := StrToIntDef(Copy(ISODateTime, 21, 3),  0); // Optional
  if (d < 1) or (d > DaysInAMonth(y, m)) then
    raise EConvertError.Create(_('Invalid day of the month: ') + IntToStr(d));
  if (m < 1) or (m > 12) then
    raise EConvertError.Create(_('Invalid month of the year: ') + IntToStr(m));
  if (h > 23) then
    raise EConvertError.Create(_('Invalid hour of the day: ') + IntToStr(h));
  if (n > 59) then
    raise EConvertError.Create(_('Invalid minute of the hour: ') + IntToStr(n));
  if (s > 59) then
    raise EConvertError.Create(_('Invalid second of the minute: ') + IntToStr(s));
  Result := EncodeDate(y, m, d) + EncodeTime(h, n, s, z);
end;

Here, the function _ is the translation function provided by GnuGettext. The string arguments passed to the EConvertError.Create method will be translated using the appropriate translation based on the user’s locale.

Now I’m actually impressed. But still no use of CreateFmt, so I asked it again “Change that code to use CreateFmt rather than Create for the error messages.”

The following code can be used to translate the error messages using GnuGettext and CreateFmt:

function IsoStringToDateTime(const ISODateTime: string): TDateTime;
const
  ISOShortLen = 19;
  ISOFullLen = 23;
var
  y, m, d, h, n, s, z: Word;
begin
  if (Length(ISODateTime) <> ISOShortLen) and (Length(ISODateTime) <> ISOFullLen) then
    raise EConvertError.CreateFmt(_('Invalid ISO date time string: %s'), [ISODateTime]);
  y := SysUtils.StrToInt(Copy(ISODateTime,  1, 4));
  m := SysUtils.StrToInt(Copy(ISODateTime,  6, 2));
  d := SysUtils.StrToInt(Copy(ISODateTime,  9, 2));
  h := SysUtils.StrToInt(Copy(ISODateTime, 12, 2));
  n := SysUtils.StrToInt(Copy(ISODateTime, 15, 2));
  s := SysUtils.StrToInt(Copy(ISODateTime, 18, 2));
  z := StrToIntDef(Copy(ISODateTime, 21, 3),  0); // Optional
  if (d < 1) or (d > DaysInAMonth(y, m)) then
    raise EConvertError.CreateFmt(_('Invalid day of the month: %d'), [d]);
  if (m < 1) or (m > 12) then
    raise EConvertError.CreateFmt(_('Invalid month of the year: %d'), [m]);
  if (h > 23) then
    raise EConvertError.CreateFmt(_('Invalid hour of the day: %d'), [h]);
  if (n > 59) then
    raise EConvertError.CreateFmt(_('Invalid minute of the hour: %d'), [n]);
  if (s > 59) then
    raise EConvertError.CreateFmt(_('Invalid second of the minute: %d'), [s]);
  Result := EncodeDate(y, m, d) + EncodeTime(h, n, s, z);
end;

Here, the function CreateFmt allows you to use format string specifiers (e.g. %d, %s) in the error message, and the values for these specifiers are passed as a list of arguments. The translated error messages will have the correct values for these format specifiers.

I haven’t tried to compile this, but it definitely looks like code I could have written and I can’t spot any bugs.