Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

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?

like image 874
Jan Doggen Avatar asked Aug 12 '14 14:08

Jan Doggen


1 Answers

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]);
like image 186
David Heffernan Avatar answered Oct 21 '22 17:10

David Heffernan