TDataModule descendant 'created' without .Create but no issues?

Question

I suddenly noticed a TDataModuleTestExchange(nil) 'constructor call' in our codebase:

procedure TDialoogConfigExchange.ButtonTestClick(Sender: TObject);
var
   lDataModuleTestExchange: TDataModuleTestExchange;
   lResult                : Boolean;
begin
   inherited;
   [snip]
   begin
      lDataModuleTestExchange := TDataModuleTestExchange(nil);  // *** HERE ***
      try
         lResult := lDataModuleTestExchange.GetCalendarFolder(EditHost.Text,EditGebruiker.Text,EditWachtwoord.Text);
         if lResult then
            ToonMelding(sExchangeTestGelukt, mtInformation, [mbOk])
         else
            ToonMelding(Meldingen.Text, mtError, [mbOK]);
      finally
         lDataModuleTestExchange.Free;
      end;
   end;
end;

So instead of TDataModuleTestExchange.**Create**(nil) this works just fine!

unit dmTestExchange;

interface

uses
  System.SysUtils, System.Classes,
  Xml.XMLDoc, Xml.XMLIntf, Xml.XMLDOM,
  TimeTellDM;

type
  TDataModuleTestExchange = class(TTimeTellDataModule)  // TDataModule descendant
  private
  public
    function GetCalendarFolder(const AExchangeServerURL,AExchangeLoginName,AExchangePass: String): Boolean;
  end;

No compiler error, no run-time issues. How come?

Charron

TButton(nil).Dragging is also safe ;) – Talkative

@Talkative So, when there is no control being dragged, TButton(nil).Dragging is True! – Johnnie

Answer 1

First of all, it is worth pointing out that the cast is spurious and serves no purpose other than to confuse. The code is equivalent to:

lDataModuleTestExchange := nil;

TDataModuleTestExchange.GetCalendarFolder is an instance method and you are calling it on a nil reference. This will result in a runtime error if the method attempts to access any fields in the instance, or call virtual methods, or indeed anything that depends upon the instance. So, it seems likely that the implementation of TDataModuleTestExchange.GetCalendarFolder does not depend upon the instance. Although you appear to get away with this here, it is clearly very poor form to write code like this.

The class should likely be re-written to declare a static class method like so:

type
  TDataModuleTestExchange = class(TTimeTellDataModule)  
  public
    class function GetCalendarFolder(const AExchangeServerURL,  
      AExchangeLoginName, AExchangePass: string): Boolean; static;
  end;

And then called like this:

lResult := TDataModuleTestExchange.GetCalendarFolder(EditHost.Text,
  EditGebruiker.Text, EditWachtwoord.Text);
if lResult then
  ToonMelding(sExchangeTestGelukt, mtInformation, [mbOk])
else
  ToonMelding(Meldingen.Text, mtError, [mbOK]);

Johnnie

Thanks, it was a simple typo. It was not intended as a class method, I had already added the .Create. Indeed, function GetCalendarFolder does not access private variables and does not call virtual methods: as my code shows TDataModuleTestExchange has none and neither does the TTimeTellDataModule = class(TDataModule) ;-) – Charron

That's not true. TDataModuleTestExchange derives from TDataModule which has both data fields and virtual methods. You really should make this a class method since it does not use any instance data and so spinning up an instance to do nothing is wasteful and misleading. In my view. – Johnnie